feat: add PostHog analytics to web and marketing apps#351
feat: add PostHog analytics to web and marketing apps#351saddlepaddle wants to merge 2 commits intomainfrom
Conversation
WalkthroughAdds PostHog analytics integration: environment vars, deployment secrets, PostHog clients and consent UI, client-side user identification, admin dashboard components, TRPC analytics router and PostHog API helpers, and related package/dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (User)
participant Marketing as Marketing App
participant Web as Web App
participant Consent as CookieConsent
participant PostHogClient as posthog-js
participant Ident as PostHogUserIdentifier
participant TRPC as TRPC Server (analyticsRouter)
participant PostHogAPI as PostHog HTTP API
Browser->>Marketing: Visit site
Marketing->>Consent: Render cookie banner
Consent->>Consent: Read ANALYTICS_CONSENT_KEY
alt consent accepted
Consent->>PostHogClient: posthog.opt_in_capturing()
else user accepts now
Browser->>Consent: Click Accept
Consent->>Consent: set ANALYTICS_CONSENT_KEY = "accepted"
Consent->>PostHogClient: posthog.opt_in_capturing()
end
opt User authenticates in Web
Browser->>Web: Login
Web->>Ident: mount PostHogUserIdentifier
Ident->>PostHogClient: posthog.identify(id, props)
end
Browser->>Marketing: User interaction (pageview/event)
Marketing->>PostHogClient: track events (api_host -> /ingest proxy)
PostHogClient->>PostHogAPI: send events
Admin->>TRPC: Request analytics (getActivationFunnel, getRecentUserEvents)
TRPC->>PostHogAPI: query endpoints (funnels, events, persons) using POSTHOG_API_KEY
PostHogAPI->>TRPC: return results
TRPC->>Admin Dashboard: structured data
Admin Dashboard->>Browser: render ActivationFunnel / StatsCard / UserEventsTable
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
saddlepaddle
force-pushed
the
wicked-hummingbird-f35f2b
branch
from
5ed0a9e to
3f57720
Compare
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy-preview.yml (1)
127-142: PostHog environment variables not passed to Vercel deployment.
POSTHOG_API_KEYandPOSTHOG_PROJECT_IDare set in the step'senvblock (available at build time), but they are not included in thevercel deploy --envflags. If these variables are needed at runtime on the deployed API server, they won't be available.Either add them to the deploy command:
VERCEL_URL=$(vercel deploy --prebuilt --token=$VERCEL_TOKEN \ --env CLERK_SECRET_KEY=$CLERK_SECRET_KEY \ --env CLERK_WEBHOOK_SECRET=$CLERK_WEBHOOK_SECRET \ --env DATABASE_URL=$DATABASE_URL \ --env DATABASE_URL_UNPOOLED=$DATABASE_URL_UNPOOLED \ --env BLOB_READ_WRITE_TOKEN=$BLOB_READ_WRITE_TOKEN \ --env NEXT_PUBLIC_WEB_URL=$NEXT_PUBLIC_WEB_URL \ --env NEXT_PUBLIC_ADMIN_URL=$NEXT_PUBLIC_ADMIN_URL \ - --env NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY=$NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY) + --env NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY=$NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY \ + --env POSTHOG_API_KEY=$POSTHOG_API_KEY \ + --env POSTHOG_PROJECT_ID=$POSTHOG_PROJECT_ID)Or configure them in Vercel project settings if they should be static across deployments.
🧹 Nitpick comments (16)
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
11-21: Email PII is sent to PostHog analytics.The component sends
Also,
user_id: user.id(line 16) is redundant sinceuser.idis already passed as the distinct ID inposthog.identify().posthog.identify(user.id, { email: user.primaryEmailAddress?.emailAddress, name: user.fullName, - user_id: user.id, });apps/admin/src/app/(dashboard)/events/[personId]/page.tsx (3)
34-45: DuplicategetEventColorfunction.This function is duplicated in
UserEventsTable.tsx(lines 42-53). Extract it to a shared utility to maintain consistency and reduce duplication.Create a shared utility, e.g., in a
utilsfolder:// utils/eventColors.ts export function getEventColor(event: string) { if (event.startsWith("$")) return "secondary"; if (event.includes("completed") || event.includes("success")) return "default"; if (event.includes("error") || event.includes("failed")) return "destructive"; return "outline"; }
107-107: Inconsistent truncation length.This uses
personId.slice(0, 16)whileUserEventsTable.tsxusesslice(0, 12). Consider using a consistent truncation length or extracting a shared utility.
187-197: Consider guarding against invalid date strings.If
event.timestampis malformed,new Date(event.timestamp)will produce an "Invalid Date", causingformat()andformatDistanceToNow()to throw or display unexpected output.const date = new Date(event.timestamp); const isValidDate = !isNaN(date.getTime()); // Then conditionally render or show fallbackapps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx (2)
24-40: Consider exporting theUserEventtype.The
UserEventinterface could be useful elsewhere (e.g., the events page). Exporting it from this component or a shared types file would improve reusability.-interface UserEvent { +export interface UserEvent {
128-128: Hardcoded event limit.The
slice(0, 10)limit is hardcoded. Consider making this configurable via props if different consumers need different limits.interface UserEventsTableProps { events: UserEvent[]; isLoading?: boolean; error?: string; showViewAll?: boolean; + maxEvents?: number; } export function UserEventsTable({ events, isLoading, error, showViewAll = true, + maxEvents = 10, }: UserEventsTableProps) { // ... - {events.slice(0, 10).map((event) => ( + {events.slice(0, maxEvents).map((event) => (.env.example (1)
29-36: PostHog variables added correctly.The environment variables are well-organized with clear comments distinguishing client-side tracking variables from server-side API variables.
Note: The static analysis tool suggests alphabetical ordering (NEXT_PUBLIC_POSTHOG_HOST before NEXT_PUBLIC_POSTHOG_KEY), but the current semantic grouping (KEY then HOST) is also reasonable.
If you prefer strict alphabetical ordering, apply this diff:
# PostHog Analytics (Client-side tracking) -NEXT_PUBLIC_POSTHOG_KEY=your_posthog_project_api_key_here NEXT_PUBLIC_POSTHOG_HOST=https://us.i.posthog.com +NEXT_PUBLIC_POSTHOG_KEY=your_posthog_project_api_key_hereapps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
18-27: Wrap localStorage writes in try-catch for robustness.Similar to the read operation,
localStorage.setItem()can throw. The banner should still hide even if storage fails.const handleAccept = () => { + try { localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted"); + } catch { + // Storage unavailable - continue anyway + } posthog.opt_in_capturing(); setShowBanner(false); }; const handleDecline = () => { + try { localStorage.setItem(ANALYTICS_CONSENT_KEY, "declined"); + } catch { + // Storage unavailable - continue anyway + } setShowBanner(false); };packages/trpc/src/router/analytics.ts (1)
257-267: Consider limiting exposed properties in searchUsers response.The
propertiesfield exposes all PostHog person properties to the admin UI. While admin-protected, consider explicitly picking safe properties to avoid accidentally exposing sensitive data in the future.return { users: persons.results.map((person) => ({ id: person.id, distinctIds: person.distinct_ids, email: person.properties.email as string | undefined, name: person.properties.name as string | undefined, createdAt: person.created_at, - properties: person.properties, + // Only include properties needed by the admin UI })), };apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx (2)
72-73: Consider using index as part of the key.If
step.namevalues are not guaranteed unique, this could cause React reconciliation issues. Consider using a composite key or index:- <div key={step.name} className="flex flex-1 items-center gap-2"> + <div key={`${step.name}-${index}`} className="flex flex-1 items-center gap-2">
81-86: Opacity may go negative for large step counts.The formula
1 - index * 0.15produces negative values whenindex > 6. CSS will clamp this to 0, but consider clamping explicitly for clarity:style={{ - opacity: 1 - index * 0.15, + opacity: Math.max(0.1, 1 - index * 0.15), }}apps/admin/src/app/(dashboard)/events/page.tsx (3)
43-43: Unused state setter - use const instead.
limitnever changes, souseStateis unnecessary overhead:- const [limit] = useState(50); + const limit = 50;
51-56: Consider debouncing the search input.Each keystroke triggers a new API call via
searchUsers. Consider debouncing to reduce server load:import { useDeferredValue } from "react"; // In component: const deferredSearchQuery = useDeferredValue(searchQuery); const { data: usersData, isLoading: isLoadingUsers } = useQuery( trpc.analytics.searchUsers.queryOptions({ search: deferredSearchQuery || undefined, limit: 20, }), );Alternatively, use a debounce hook or library for more control over the delay.
Also applies to: 116-118
29-40: Event color matching is case-sensitive.The
includes()checks are case-sensitive, so events like"Error"or"FAILED"won't match. Consider case-insensitive matching if event naming isn't strictly controlled:function getEventColor(event: string) { + const lowerEvent = event.toLowerCase(); if (event.startsWith("$")) { return "secondary"; } - if (event.includes("completed") || event.includes("success")) { + if (lowerEvent.includes("completed") || lowerEvent.includes("success")) { return "default"; } - if (event.includes("error") || event.includes("failed")) { + if (lowerEvent.includes("error") || lowerEvent.includes("failed")) { return "destructive"; } return "outline"; }apps/admin/src/app/(dashboard)/page.tsx (1)
117-125: Type assertion bypasses type safety.The
ascast trusts the API response shape without validation. If the API response structure changes, this will fail silently at runtime. Consider using Zod or a similar library for runtime validation:import { z } from "zod"; const funnelResponseSchema = z.object({ results: z.array(z.object({ count: z.number().optional(), name: z.string().optional(), order: z.number().optional(), conversion_rate: z.number().optional(), })).optional(), }); // Then: const parsed = funnelResponseSchema.safeParse(funnelData); if (!parsed.success) { return defaultSteps; } const data = parsed.data;As per coding guidelines, prioritize type safety in TypeScript code.
packages/trpc/src/lib/posthog.ts (1)
63-73: Consider adding request timeouts.The
fetchcalls have no timeout configured, which could cause hanging requests if the PostHog API is slow or unresponsive. Consider usingAbortSignal.timeout():const response = await fetch(url, { // ...existing options signal: AbortSignal.timeout(30000), // 30 second timeout });Also applies to: 99-106, 140-147, 186-193
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.env.example(1 hunks).github/workflows/deploy-preview.yml(3 hunks).github/workflows/deploy-production.yml(3 hunks)apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx(1 hunks)apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts(1 hunks)apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx(1 hunks)apps/admin/src/app/(dashboard)/components/StatsCard/index.ts(1 hunks)apps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx(1 hunks)apps/admin/src/app/(dashboard)/components/UserEventsTable/index.ts(1 hunks)apps/admin/src/app/(dashboard)/events/[personId]/page.tsx(1 hunks)apps/admin/src/app/(dashboard)/events/page.tsx(1 hunks)apps/admin/src/app/(dashboard)/page.tsx(1 hunks)apps/marketing/next.config.ts(1 hunks)apps/marketing/package.json(1 hunks)apps/marketing/src/app/layout.tsx(2 hunks)apps/marketing/src/components/CookieConsent/CookieConsent.tsx(1 hunks)apps/marketing/src/components/CookieConsent/index.ts(1 hunks)apps/marketing/src/env.ts(2 hunks)apps/marketing/src/instrumentation-client.ts(1 hunks)apps/web/next.config.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/src/app/layout.tsx(2 hunks)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/web/src/components/PostHogUserIdentifier/index.ts(1 hunks)apps/web/src/env.ts(2 hunks)apps/web/src/instrumentation-client.ts(1 hunks)packages/shared/src/constants.ts(1 hunks)packages/trpc/package.json(1 hunks)packages/trpc/src/env.ts(1 hunks)packages/trpc/src/lib/posthog.ts(1 hunks)packages/trpc/src/root.ts(2 hunks)packages/trpc/src/router/analytics.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/admin/src/app/(dashboard)/components/UserEventsTable/index.tsapps/web/src/components/PostHogUserIdentifier/index.tsapps/admin/src/app/(dashboard)/components/ActivationFunnel/index.tsapps/marketing/src/app/layout.tsxapps/marketing/src/components/CookieConsent/index.tspackages/shared/src/constants.tsapps/web/src/instrumentation-client.tsapps/web/package.jsonapps/web/src/env.tspackages/trpc/package.jsonapps/marketing/package.jsonapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/marketing/next.config.tspackages/trpc/src/root.tsapps/web/src/app/layout.tsxapps/admin/src/app/(dashboard)/events/page.tsxapps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsxapps/admin/src/app/(dashboard)/components/StatsCard/index.tsapps/marketing/src/instrumentation-client.tsapps/admin/src/app/(dashboard)/page.tsxapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/admin/src/app/(dashboard)/events/[personId]/page.tsxapps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsxapps/marketing/src/env.tspackages/trpc/src/router/analytics.tsapps/web/next.config.tspackages/trpc/src/env.tspackages/trpc/src/lib/posthog.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/admin/src/app/(dashboard)/components/UserEventsTable/index.tsapps/web/src/components/PostHogUserIdentifier/index.tsapps/admin/src/app/(dashboard)/components/ActivationFunnel/index.tsapps/marketing/src/app/layout.tsxapps/marketing/src/components/CookieConsent/index.tspackages/shared/src/constants.tsapps/web/src/instrumentation-client.tsapps/web/src/env.tsapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/marketing/next.config.tspackages/trpc/src/root.tsapps/web/src/app/layout.tsxapps/admin/src/app/(dashboard)/events/page.tsxapps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsxapps/admin/src/app/(dashboard)/components/StatsCard/index.tsapps/marketing/src/instrumentation-client.tsapps/admin/src/app/(dashboard)/page.tsxapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/admin/src/app/(dashboard)/events/[personId]/page.tsxapps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsxapps/marketing/src/env.tspackages/trpc/src/router/analytics.tsapps/web/next.config.tspackages/trpc/src/env.tspackages/trpc/src/lib/posthog.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/admin/src/app/(dashboard)/components/UserEventsTable/index.tsapps/web/src/components/PostHogUserIdentifier/index.tsapps/admin/src/app/(dashboard)/components/ActivationFunnel/index.tsapps/marketing/src/components/CookieConsent/index.tsapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsxapps/admin/src/app/(dashboard)/components/StatsCard/index.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to **/components/**/*.{ts,tsx} : Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Applied to files:
apps/admin/src/app/(dashboard)/components/UserEventsTable/index.tsapps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to **/components/**/*.{ts,tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts
📚 Learning: 2025-12-12T05:45:09.673Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.673Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in apps/desktop/src/main/index.ts with override: true before any imports
Applied to files:
.env.examplepackages/trpc/src/env.ts
🧬 Code graph analysis (12)
apps/marketing/src/app/layout.tsx (2)
apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
CookieConsent(8-73)apps/marketing/src/components/CookieConsent/index.ts (1)
CookieConsent(1-1)
apps/web/src/instrumentation-client.ts (2)
apps/marketing/src/env.ts (1)
env(5-37)apps/web/src/env.ts (1)
env(5-46)
apps/marketing/src/components/CookieConsent/CookieConsent.tsx (2)
apps/marketing/src/components/CookieConsent/index.ts (1)
CookieConsent(1-1)packages/shared/src/constants.ts (1)
ANALYTICS_CONSENT_KEY(13-13)
packages/trpc/src/root.ts (1)
packages/trpc/src/router/analytics.ts (1)
analyticsRouter(12-297)
apps/web/src/app/layout.tsx (1)
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-24)
apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx (3)
apps/admin/src/app/(dashboard)/components/StatsCard/index.ts (1)
StatsCard(1-1)packages/ui/src/components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx (3)
apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts (1)
ActivationFunnel(1-1)packages/ui/src/components/ui/card.tsx (5)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)CardContent(91-91)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/marketing/src/instrumentation-client.ts (3)
apps/marketing/src/env.ts (1)
env(5-37)apps/web/src/env.ts (1)
env(5-46)packages/shared/src/constants.ts (1)
ANALYTICS_CONSENT_KEY(13-13)
apps/admin/src/app/(dashboard)/page.tsx (3)
apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx (1)
StatsCard(18-77)apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx (1)
ActivationFunnel(25-120)apps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx (1)
UserEventsTable(55-161)
apps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx (4)
packages/ui/src/components/ui/card.tsx (5)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)CardContent(91-91)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)packages/ui/src/components/ui/table.tsx (5)
TableHeader(109-109)TableRow(113-113)TableHead(112-112)TableBody(110-110)TableCell(114-114)packages/ui/src/components/ui/badge.tsx (1)
Badge(46-46)
packages/trpc/src/env.ts (4)
apps/marketing/src/env.ts (1)
env(5-37)apps/web/src/env.ts (1)
env(5-46)apps/admin/src/env.ts (1)
env(5-37)apps/api/src/env.ts (1)
env(4-22)
packages/trpc/src/lib/posthog.ts (1)
packages/trpc/src/env.ts (1)
env(4-13)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 32-32: [UnorderedKey] The NEXT_PUBLIC_POSTHOG_HOST key should go before the NEXT_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
⏰ 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 API
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Admin
- GitHub Check: Build
🔇 Additional comments (34)
apps/web/package.json (1)
30-30: LGTM!The
posthog-jsdependency addition aligns with the PR's goal of adding client-side PostHog analytics to the web app.packages/trpc/package.json (1)
23-23: LGTM!The
posthog-nodedependency is appropriate for server-side PostHog integration in the TRPC layer..github/workflows/deploy-preview.yml (1)
210-219:NEXT_PUBLIC_POSTHOG_KEYnot passed to Vercel deploy command.Same issue: the variable is available at build time but not at runtime unless passed via
--envor configured in Vercel project settings. Since it's aNEXT_PUBLIC_*variable, it gets inlined at build time, so this should be fine for client-side usage. No action needed if the build step correctly picks it up.Verify that
NEXT_PUBLIC_POSTHOG_KEYis correctly embedded in the client bundle duringvercel build. If PostHog initialization fails in preview deployments, this would be the cause.apps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx (1)
55-160: LGTM!The component handles loading, error, and empty states well. The UI structure is clean with proper use of Card, Table, and Badge components.
apps/admin/src/app/(dashboard)/components/UserEventsTable/index.ts (1)
1-1: Barrel export looks correct and matches the component folder convention.apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts (1)
1-1: Barrel export looks correct and matches the component folder convention.packages/trpc/src/env.ts (1)
1-13: Good: schema-validating PostHog server envs; ensure callers fail loudly when unset.
SincePOSTHOG_API_KEY/POSTHOG_PROJECT_IDare optional, make sure any analytics router logic that requires them explicitly errors/short-circuits rather than partially functioning.apps/marketing/src/app/layout.tsx (1)
8-65: CookieConsent placement is fine; ensure it’s the single source of truth for marketing tracking enablement.
Given the consent key issue above, confirm the intended persistence behavior across app/marketing origins.apps/web/src/components/PostHogUserIdentifier/index.ts (1)
1-1: Barrel export looks correct and keeps imports clean.apps/marketing/package.json (1)
23-31: No action needed regardingposthog-js@^1.306.1—no breaking changes documented foropt_in_capturingand no known security advisories affecting this version.apps/web/src/app/layout.tsx (1)
7-65: Verify component implementation and consent/feature flag status.The review's concern about PostHogUserIdentifier potentially exposing user PII (email/name) through
identify()calls cannot be fully verified without access to the component's implementation. If this component callsposthog.identify()with sensitive user data, the review is valid: ensure either (1) explicit user consent is in place before identifying users, (2) the component is gated behind a feature flag or privacy setting, or (3) the identify payload is documented. Additionally, confirm PostHog is initialized before this component'suseEffectruns to avoid race conditions. The location cannot be pinpointed without access to the component definition.apps/marketing/src/components/CookieConsent/index.ts (1)
1-1: LGTM! Barrel export follows guidelines.The re-export follows the component structure guidelines (ComponentName/index.ts barrel export pattern).
apps/admin/src/app/(dashboard)/components/StatsCard/index.ts (1)
1-1: LGTM! Consistent barrel export.The re-export matches the pattern used by other dashboard components (ActivationFunnel, UserEventsTable).
packages/trpc/src/root.ts (1)
4-4: LGTM! Analytics router integration is correct.The import and router registration follow the established pattern for other routers (admin, organization, repository, task, user). Type inference will automatically include the analytics namespace in the exported
AppRoutertype.Also applies to: 13-13
apps/web/next.config.ts (1)
13-22: LGTM! PostHog proxy rewrites are correctly configured.The rewrites properly route
/ingesttraffic to PostHog's US hosts to bypass ad blockers. The more specific/ingest/static/route is correctly ordered before the general/ingest/catch-all route, and the destinations align with theapi_host: "/ingest"configuration in the instrumentation client.apps/marketing/src/instrumentation-client.ts (1)
1-29: LGTM! GDPR-compliant PostHog initialization.The implementation correctly follows GDPR requirements with opt-out by default tracking. The consent check via localStorage using the shared
ANALYTICS_CONSENT_KEYconstant enables consistent consent management across subdomains.Key features:
opt_out_capturing_by_default: trueensures no tracking without consent- Cross-subdomain cookie persistence for unified tracking
/ingestproxy routing to bypass ad blockers- App context registration in the loaded callback
.github/workflows/deploy-production.yml (1)
134-134: LGTM! Client-side PostHog key correctly configured.
NEXT_PUBLIC_POSTHOG_KEYis correctly set in the build environment for both Web (line 134) and Marketing (line 180) deployments. SinceNEXT_PUBLIC_*variables are embedded in the client bundle at build time, they don't need to be passed with--envflags to the deploy command.Also applies to: 180-180
apps/web/src/instrumentation-client.ts (1)
1-22: LGTM! Web app tracking initialization is correct.The PostHog initialization for the web app appropriately tracks by default (no
opt_out_capturing_by_default), which aligns with the PR objectives for authenticated user tracking. This intentionally differs from the marketing app's GDPR-compliant opt-out approach.Configuration highlights:
- Automatic pageview/pageleave/exception tracking
- Cross-subdomain cookie persistence for unified tracking
/ingestproxy to bypass ad blockers- App context registration with
app_name: "web"apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
29-72: LGTM!The UI implementation is clean with proper responsive design, accessible button types, and smooth framer-motion animations. The consent text clearly explains the tracking behavior.
apps/marketing/src/env.ts (2)
20-24: LGTM!The PostHog environment variables are properly configured with
NEXT_PUBLIC_POSTHOG_KEYas required andNEXT_PUBLIC_POSTHOG_HOSTwith a sensible default. This aligns with the web app configuration and the Next.js rewrites.
33-34: LGTM!Runtime environment mappings correctly expose both PostHog variables to the client bundle.
apps/marketing/next.config.ts (1)
14-23: LGTM!The rewrite rules correctly proxy PostHog traffic through
/ingestto bypass ad blockers. The more specific/ingest/static/:path*rule is correctly placed before the catch-all/ingest/:path*rule.apps/web/src/env.ts (1)
25-29: LGTM!PostHog environment variables are consistently configured with the marketing app, using the same schema validation and default values.
Also applies to: 40-41
packages/trpc/src/router/analytics.ts (5)
43-80: LGTM!The trend query correctly uses a CTE to aggregate weekly active users. The
weeksparameter is safely bounded by Zod validation (1-12).
85-118: LGTM!The activation funnel query cleanly uses PostHog's structured
FunnelsQueryAPI with appropriate defaults and test account filtering.
123-175: LGTM!The retention cohort query is well-structured with appropriate CTEs for first-seen and weekly activity tracking.
180-237: LGTM!User event endpoints properly limit results and are appropriately protected by admin procedures. The response mapping cleanly extracts relevant fields.
272-296: LGTM!The user stats query is clean and efficiently retrieves multiple time-windowed counts in a single query with proper null handling.
apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx (1)
25-119: LGTM - Well-structured component.The component handles loading, error, and data states cleanly. The conversion rate color coding and funnel visualization are well implemented.
apps/admin/src/app/(dashboard)/events/page.tsx (1)
42-221: LGTM - Solid events page implementation.The page handles loading, error, and empty states well. The user search panel and events table are cleanly implemented with proper data fallbacks.
apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx (1)
18-77: LGTM - Clean stats card implementation.The component handles loading states with appropriate skeletons, and the trend direction logic with color-coded icons is well implemented.
apps/admin/src/app/(dashboard)/page.tsx (2)
137-144: Good fallback handling for step names.The nullish coalescing chain
stepNames[index] ?? step.name ?? \Step ${index + 1}`` gracefully handles cases where the API returns more steps than expected.
11-105: LGTM - Well-organized dashboard page.The page effectively composes multiple analytics queries and transforms data for child components. Loading and error states are properly propagated.
packages/trpc/src/lib/posthog.ts (1)
83-122: LGTM - Well-typed API wrapper functions.The functions have explicit return types and handle errors consistently. The type definitions provide good structure for the PostHog API responses.
Also applies to: 124-163, 165-214
| useEffect(() => { | ||
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | ||
| if (consent === null) { | ||
| setShowBanner(true); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Add error handling for localStorage access.
localStorage.getItem() can throw in private browsing mode or when storage is disabled. This would prevent the banner from showing and cause a runtime error.
useEffect(() => {
+ try {
const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY);
if (consent === null) {
setShowBanner(true);
}
+ } catch {
+ // localStorage unavailable (private browsing, etc.) - show banner
+ setShowBanner(true);
+ }
}, []);📝 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.
| useEffect(() => { | |
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | |
| if (consent === null) { | |
| setShowBanner(true); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| try { | |
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | |
| if (consent === null) { | |
| setShowBanner(true); | |
| } | |
| } catch { | |
| // localStorage unavailable (private browsing, etc.) - show banner | |
| setShowBanner(true); | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In apps/marketing/src/components/CookieConsent/CookieConsent.tsx around lines 11
to 16, accessing localStorage.getItem() can throw (e.g., in private browsing or
when storage is disabled); wrap the localStorage access in a try/catch (and
guard for typeof window !== 'undefined' if needed), fall back to treating errors
as "no consent" by calling setShowBanner(true) when an exception occurs, and
optionally log a warning to the console so the component doesn't crash and the
banner still displays.
| export const THEME_STORAGE_KEY = "superset-theme"; | ||
|
|
||
| // Analytics | ||
| export const ANALYTICS_CONSENT_KEY = "superset-analytics-consent"; | ||
|
|
There was a problem hiding this comment.
“Cross-subdomain persistence” won’t happen with just a shared key if storage is localStorage.
Same key name helps consistency, but localStorage is per-origin; marketing + web subdomains won’t share consent state. If you truly need cross-subdomain consent, use a cookie with Domain=.superset.sh (or server-side persistence) and read it from both apps.
🤖 Prompt for AI Agents
In packages/shared/src/constants.ts around lines 10 to 14 the consent key is
defined for use with client storage but using a shared key alone won’t enable
cross-subdomain persistence because localStorage is origin-scoped; to fix this,
replace or augment localStorage usage with a cookie-based persistence (or
server-side store) for consent: create/read the consent cookie with
Domain=.superset.sh, set Secure and SameSite as appropriate, and ensure write
operations set the domain flag so both marketing and web subdomains can read it;
keep the constant for the cookie name and add a fallback to localStorage if
cookies are unavailable.
| export async function queryPostHog<T = PostHogQueryResult>( | ||
| query: PostHogQuery, | ||
| ): Promise<T> { | ||
| const response = await fetch( | ||
| `${POSTHOG_API_BASE}/projects/${env.POSTHOG_PROJECT_ID}/query`, | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${env.POSTHOG_API_KEY}`, | ||
| }, | ||
| body: JSON.stringify({ query }), | ||
| }, | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| const error = await response.text(); | ||
| throw new Error(`PostHog API error: ${response.status} - ${error}`); | ||
| } | ||
|
|
||
| return response.json() as Promise<T>; | ||
| } |
There was a problem hiding this comment.
Missing validation for required environment variables.
POSTHOG_API_KEY and POSTHOG_PROJECT_ID are defined as optional in env.ts, but this function uses them without validation. If either is undefined, the API calls will fail with malformed URLs or authorization headers:
export async function queryPostHog<T = PostHogQueryResult>(
query: PostHogQuery,
): Promise<T> {
+ if (!env.POSTHOG_API_KEY || !env.POSTHOG_PROJECT_ID) {
+ throw new Error("PostHog configuration missing: POSTHOG_API_KEY and POSTHOG_PROJECT_ID are required");
+ }
+
const response = await fetch(Apply the same check to getPersons, getPersonEvents, and getRecentEvents, or extract to a shared validation helper.
📝 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.
| export async function queryPostHog<T = PostHogQueryResult>( | |
| query: PostHogQuery, | |
| ): Promise<T> { | |
| const response = await fetch( | |
| `${POSTHOG_API_BASE}/projects/${env.POSTHOG_PROJECT_ID}/query`, | |
| { | |
| method: "POST", | |
| headers: { | |
| "Content-Type": "application/json", | |
| Authorization: `Bearer ${env.POSTHOG_API_KEY}`, | |
| }, | |
| body: JSON.stringify({ query }), | |
| }, | |
| ); | |
| if (!response.ok) { | |
| const error = await response.text(); | |
| throw new Error(`PostHog API error: ${response.status} - ${error}`); | |
| } | |
| return response.json() as Promise<T>; | |
| } | |
| export async function queryPostHog<T = PostHogQueryResult>( | |
| query: PostHogQuery, | |
| ): Promise<T> { | |
| if (!env.POSTHOG_API_KEY || !env.POSTHOG_PROJECT_ID) { | |
| throw new Error("PostHog configuration missing: POSTHOG_API_KEY and POSTHOG_PROJECT_ID are required"); | |
| } | |
| const response = await fetch( | |
| `${POSTHOG_API_BASE}/projects/${env.POSTHOG_PROJECT_ID}/query`, | |
| { | |
| method: "POST", | |
| headers: { | |
| "Content-Type": "application/json", | |
| Authorization: `Bearer ${env.POSTHOG_API_KEY}`, | |
| }, | |
| body: JSON.stringify({ query }), | |
| }, | |
| ); | |
| if (!response.ok) { | |
| const error = await response.text(); | |
| throw new Error(`PostHog API error: ${response.status} - ${error}`); | |
| } | |
| return response.json() as Promise<T>; | |
| } |
🤖 Prompt for AI Agents
In packages/trpc/src/lib/posthog.ts around lines 60 to 81, the function uses
env.POSTHOG_API_KEY and env.POSTHOG_PROJECT_ID which are optional in env.ts
without validating them first; add a validation step that throws a clear error
if either env variable is missing (or return early) before constructing the URL
or headers, and refactor this validation into a small shared helper (e.g.,
ensurePostHogEnv or validatePostHogConfig) that is called from queryPostHog as
well as getPersons, getPersonEvents, and getRecentEvents so all functions fail
fast with an explicit message when the required env vars are not set.
| getQualityWAU: adminProcedure.query(async () => { | ||
| const result = await queryPostHog({ | ||
| kind: "DataVisualizationNode", | ||
| source: { | ||
| kind: "HogQLQuery", | ||
| query: ` | ||
| SELECT count(DISTINCT person_id) as quality_wau | ||
| FROM events | ||
| WHERE timestamp >= now() - INTERVAL 7 DAY | ||
| AND event IN ('task_started', 'task_completed') | ||
| GROUP BY person_id | ||
| HAVING count(DISTINCT toDate(timestamp)) >= 3 | ||
| `, | ||
| }, | ||
| }); | ||
|
|
||
| // Return count or 0 if no results | ||
| const results = result.results as Array<[number]>; | ||
| return { | ||
| count: results.length, | ||
| period: "7d", | ||
| }; | ||
| }), |
There was a problem hiding this comment.
Query returns incorrect result - GROUP BY prevents aggregation.
The query groups by person_id and then counts DISTINCT person_id, which will return 1 for each qualifying person rather than a single total count. The results.length workaround is used, but the query should be restructured.
getQualityWAU: adminProcedure.query(async () => {
const result = await queryPostHog({
kind: "DataVisualizationNode",
source: {
kind: "HogQLQuery",
query: `
- SELECT count(DISTINCT person_id) as quality_wau
- FROM events
- WHERE timestamp >= now() - INTERVAL 7 DAY
- AND event IN ('task_started', 'task_completed')
- GROUP BY person_id
- HAVING count(DISTINCT toDate(timestamp)) >= 3
+ SELECT count(*) as quality_wau
+ FROM (
+ SELECT person_id
+ FROM events
+ WHERE timestamp >= now() - INTERVAL 7 DAY
+ AND event IN ('task_started', 'task_completed')
+ GROUP BY person_id
+ HAVING count(DISTINCT toDate(timestamp)) >= 3
+ )
`,
},
});
- // Return count or 0 if no results
- const results = result.results as Array<[number]>;
+ const results = result.results as Array<[number]>;
return {
- count: results.length,
+ count: results[0]?.[0] ?? 0,
period: "7d",
};
}),📝 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.
| getQualityWAU: adminProcedure.query(async () => { | |
| const result = await queryPostHog({ | |
| kind: "DataVisualizationNode", | |
| source: { | |
| kind: "HogQLQuery", | |
| query: ` | |
| SELECT count(DISTINCT person_id) as quality_wau | |
| FROM events | |
| WHERE timestamp >= now() - INTERVAL 7 DAY | |
| AND event IN ('task_started', 'task_completed') | |
| GROUP BY person_id | |
| HAVING count(DISTINCT toDate(timestamp)) >= 3 | |
| `, | |
| }, | |
| }); | |
| // Return count or 0 if no results | |
| const results = result.results as Array<[number]>; | |
| return { | |
| count: results.length, | |
| period: "7d", | |
| }; | |
| }), | |
| getQualityWAU: adminProcedure.query(async () => { | |
| const result = await queryPostHog({ | |
| kind: "DataVisualizationNode", | |
| source: { | |
| kind: "HogQLQuery", | |
| query: ` | |
| SELECT count(*) as quality_wau | |
| FROM ( | |
| SELECT person_id | |
| FROM events | |
| WHERE timestamp >= now() - INTERVAL 7 DAY | |
| AND event IN ('task_started', 'task_completed') | |
| GROUP BY person_id | |
| HAVING count(DISTINCT toDate(timestamp)) >= 3 | |
| ) | |
| `, | |
| }, | |
| }); | |
| const results = result.results as Array<[number]>; | |
| return { | |
| count: results[0]?.[0] ?? 0, | |
| period: "7d", | |
| }; | |
| }), |
🤖 Prompt for AI Agents
In packages/trpc/src/router/analytics.ts around lines 16 to 38 the HogQL query
groups by person_id and then SELECTs count(DISTINCT person_id), which produces
one row per person instead of a single aggregated total; replace the query with
a single aggregation that returns one row (for example SELECT count(*) AS
quality_wau FROM (SELECT person_id FROM events WHERE timestamp >= now() -
INTERVAL 7 DAY AND event IN ('task_started','task_completed') GROUP BY person_id
HAVING count(DISTINCT toDate(timestamp)) >= 3) AS sub) so the API receives a
single numeric count, then update the response parsing to read that scalar field
(e.g., result.results[0].quality_wau or equivalent) instead of using
results.length and return that as count with period "7d".
- Add posthog-js to web and marketing apps - Create instrumentation-client.ts for both apps with auto pageview/pageleave tracking - Add PostHogUserIdentifier component for web app (syncs Clerk user to PostHog) - Add CookieConsent component for marketing app (GDPR opt-out by default) - Configure Next.js rewrites for /ingest proxy (bypasses ad blockers) - Add ANALYTICS_CONSENT_KEY to shared constants for cross-app persistence - Add NEXT_PUBLIC_POSTHOG_KEY and NEXT_PUBLIC_POSTHOG_HOST env vars 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
saddlepaddle
force-pushed
the
wicked-hummingbird-f35f2b
branch
from
3f57720 to
b3748b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/deploy-preview.yml (1)
279-286: Missing runtime environment variable for marketing deployment.
NEXT_PUBLIC_POSTHOG_KEYis set in the build environment (line 279) but is not passed to thevercel deploycommand with--envflags (lines 283-286). Without this, the marketing app won't have access to the PostHog key at runtime, breaking the PostHog initialization inapps/marketing/src/instrumentation-client.tsand the cookie consent banner functionality.Apply this diff to add the missing runtime environment variable:
run: | vercel pull --yes --environment=preview --token=$VERCEL_TOKEN vercel build --token=$VERCEL_TOKEN VERCEL_URL=$(vercel deploy --prebuilt --token=$VERCEL_TOKEN \ - --env CLERK_SECRET_KEY=$CLERK_SECRET_KEY) + --env CLERK_SECRET_KEY=$CLERK_SECRET_KEY \ + --env NEXT_PUBLIC_POSTHOG_KEY=$NEXT_PUBLIC_POSTHOG_KEY) vercel alias $VERCEL_URL ${{ env.MARKETING_ALIAS }} --scope=$VERCEL_ORG_ID --token=$VERCEL_TOKEN echo "vercel_url=$VERCEL_URL" >> $GITHUB_OUTPUT.github/workflows/deploy-production.yml (1)
187-192: Missing runtime environment variable for marketing production deployment.
NEXT_PUBLIC_POSTHOG_KEYis set in the build environment (line 187) but is not passed to thevercel deploycommand with--envflags (lines 191-192). This mirrors the same issue in the preview workflow and will break PostHog initialization in production.Apply this diff to add the missing runtime environment variable:
run: | vercel pull --yes --environment=production --token=$VERCEL_TOKEN vercel build --prod --token=$VERCEL_TOKEN vercel deploy --prod --prebuilt --token=$VERCEL_TOKEN \ - --env CLERK_SECRET_KEY=$CLERK_SECRET_KEY + --env CLERK_SECRET_KEY=$CLERK_SECRET_KEY \ + --env NEXT_PUBLIC_POSTHOG_KEY=$NEXT_PUBLIC_POSTHOG_KEY
♻️ Duplicate comments (9)
apps/admin/src/app/(dashboard)/events/[personId]/page.tsx (1)
34-45: Extract duplicated getEventColor to a shared utility.This function is an exact duplicate of the one in
events/page.tsx. Refer to the earlier comment on that file for the recommended refactoring approach..github/workflows/deploy-production.yml (1)
80-96: LGTM - PostHog API credentials properly configured for production.Both environment variables and runtime deployment flags are correctly set for the API deployment. The PostHog API credentials will be available at runtime for the analytics router.
packages/shared/src/constants.ts (1)
22-23: Cross-subdomain persistence requires cookies, not localStorage.The constant name and PR objectives suggest cross-subdomain consent sharing, but
localStorageis origin-scoped and won't work acrossmarketing.superset.shandweb.superset.sh. If cross-subdomain persistence is required, implement cookie-based storage withDomain=.superset.sh.apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
11-16: Add error handling for localStorage access.
localStorage.getItem()can throw in private browsing mode or when storage is disabled. Without error handling, this will prevent the banner from rendering and cause a runtime error.useEffect(() => { + try { const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); if (consent === null) { setShowBanner(true); } + } catch { + // localStorage unavailable - show banner as fallback + setShowBanner(true); + } }, []);packages/trpc/src/router/analytics.ts (1)
16-38: Query structure produces incorrect results.The query groups by
person_idand then countsDISTINCT person_id, returning one row per person instead of a single total. The code works around this withresults.length, but the query should be restructured for correctness.getQualityWAU: adminProcedure.query(async () => { const result = await queryPostHog({ kind: "DataVisualizationNode", source: { kind: "HogQLQuery", query: ` - SELECT count(DISTINCT person_id) as quality_wau - FROM events - WHERE timestamp >= now() - INTERVAL 7 DAY - AND event IN ('task_started', 'task_completed') - GROUP BY person_id - HAVING count(DISTINCT toDate(timestamp)) >= 3 + SELECT count(*) as quality_wau + FROM ( + SELECT person_id + FROM events + WHERE timestamp >= now() - INTERVAL 7 DAY + AND event IN ('task_started', 'task_completed') + GROUP BY person_id + HAVING count(DISTINCT toDate(timestamp)) >= 3 + ) `, }, }); - const results = result.results as Array<[number]>; + const results = result.results as Array<[number]>; return { - count: results.length, + count: results[0]?.[0] ?? 0, period: "7d", }; }),packages/trpc/src/lib/posthog.ts (4)
60-81: Missing validation for required environment variables.
env.POSTHOG_API_KEYandenv.POSTHOG_PROJECT_IDare defined as optional inpackages/trpc/src/env.tsbut used without validation. If undefined, API calls will fail with malformed URLs (projects/undefined/query) or authorization headers (Bearer undefined).Add validation at the start of the function:
export async function queryPostHog<T = PostHogQueryResult>( query: PostHogQuery, ): Promise<T> { + if (!env.POSTHOG_API_KEY || !env.POSTHOG_PROJECT_ID) { + throw new Error("PostHog configuration missing: POSTHOG_API_KEY and POSTHOG_PROJECT_ID are required"); + } + const response = await fetch(Better yet, extract to a shared helper called at module load or in each function.
83-122: Missing environment validation.Same issue as
queryPostHog-env.POSTHOG_API_KEYandenv.POSTHOG_PROJECT_IDare used without validation.Apply the same validation pattern or use a shared helper.
124-163: Missing environment validation.Same issue as
queryPostHog-env.POSTHOG_API_KEYandenv.POSTHOG_PROJECT_IDare used without validation.Apply the same validation pattern or use a shared helper.
165-214: Missing environment validation.Same issue as
queryPostHog-env.POSTHOG_API_KEYandenv.POSTHOG_PROJECT_IDare used without validation.Apply the same validation pattern or use a shared helper.
Consider extracting validation to a shared function:
function ensurePostHogConfig(): { apiKey: string; projectId: string } { if (!env.POSTHOG_API_KEY || !env.POSTHOG_PROJECT_ID) { throw new Error("PostHog configuration missing: POSTHOG_API_KEY and POSTHOG_PROJECT_ID are required"); } return { apiKey: env.POSTHOG_API_KEY, projectId: env.POSTHOG_PROJECT_ID, }; }Then call it at the start of each function.
🧹 Nitpick comments (5)
apps/admin/src/app/(dashboard)/events/page.tsx (2)
29-40: Extract duplicated getEventColor to a shared utility.This function is duplicated in both
events/page.tsxandevents/[personId]/page.tsx. Extract it to a shared utilities file to follow DRY principles.Create a new utility file (e.g.,
apps/admin/src/app/(dashboard)/utils/eventUtils.ts):export function getEventColor(event: string) { if (event.startsWith("$")) { return "secondary" as const; } if (event.includes("completed") || event.includes("success")) { return "default" as const; } if (event.includes("error") || event.includes("failed")) { return "destructive" as const; } return "outline" as const; }Then import it in both files:
import { getEventColor } from "@/app/(dashboard)/utils/eventUtils";
113-118: Consider debouncing the search input.The search input triggers a query on every keystroke, which could lead to excessive API calls. Consider adding debouncing to improve performance.
You can use a debounced value with a custom hook or library like
use-debounce:import { useDebouncedValue } from '@superset/hooks'; // or 'use-debounce' // In component: const [searchQuery, setSearchQuery] = useState(""); const [debouncedSearch] = useDebouncedValue(searchQuery, 300); const { data: usersData, isLoading: isLoadingUsers } = useQuery( trpc.analytics.searchUsers.queryOptions({ search: debouncedSearch || undefined, limit: 20, }), );apps/admin/src/app/(dashboard)/page.tsx (1)
110-145: Improve type safety in transformFunnelData.The function uses
unknownwith type assertion without runtime validation, which could lead to issues if the API shape changes. Consider using a type guard or Zod schema validation.Option 1: Add runtime type guards:
function transformFunnelData( funnelData: unknown, ): Array<{ name: string; count: number; conversionRate?: number }> { if (!funnelData || typeof funnelData !== 'object') { return getDefaultSteps(); } const data = funnelData as { results?: unknown }; if (!data.results || !Array.isArray(data.results)) { return getDefaultSteps(); } const stepNames = ["Signup", "Download", "First Task", "Completed"]; return data.results.map((step: unknown, index) => { const s = step as { count?: number; name?: string; conversion_rate?: number }; return { name: stepNames[index] ?? s.name ?? `Step ${index + 1}`, count: s.count ?? 0, conversionRate: index > 0 ? (s.conversion_rate ?? 0) * 100 : undefined, }; }); } function getDefaultSteps() { return [ { name: "Signup", count: 0 }, { name: "Download", count: 0 }, { name: "First Task", count: 0 }, { name: "Completed", count: 0 }, ]; }Option 2: If Zod is available in the project, define a schema for the PostHog response shape.
apps/admin/src/app/(dashboard)/events/[personId]/page.tsx (1)
199-210: Consider a more user-friendly properties display.While
JSON.stringifyworks, it could be difficult to read for complex nested objects. Consider a collapsible JSON viewer component for better UX.For a future improvement, you could use a library like
react-json-viewor create a simple recursive component:<details className="text-sm"> <summary className="cursor-pointer text-muted-foreground"> View properties </summary> <pre className="mt-2 overflow-auto rounded bg-muted p-2 text-xs"> {JSON.stringify(event.properties, null, 2)} </pre> </details>This is a nice-to-have enhancement and not critical for the current implementation.
.env.example (1)
31-37: LGTM - PostHog environment variables added correctly.The environment variables are properly structured with client-side (
NEXT_PUBLIC_*) and server-side keys. The static analysis warning about key ordering is purely stylistic and can be safely ignored unless the team enforces alphabetical ordering.If you prefer alphabetical ordering, you can swap lines 32-33:
# PostHog Analytics (Client-side tracking) -NEXT_PUBLIC_POSTHOG_KEY=your_posthog_project_api_key_here NEXT_PUBLIC_POSTHOG_HOST=https://us.i.posthog.com +NEXT_PUBLIC_POSTHOG_KEY=your_posthog_project_api_key_here
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.env.example(1 hunks).github/workflows/deploy-preview.yml(4 hunks).github/workflows/deploy-production.yml(4 hunks)apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx(1 hunks)apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts(1 hunks)apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx(1 hunks)apps/admin/src/app/(dashboard)/components/StatsCard/index.ts(1 hunks)apps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx(1 hunks)apps/admin/src/app/(dashboard)/components/UserEventsTable/index.ts(1 hunks)apps/admin/src/app/(dashboard)/events/[personId]/page.tsx(1 hunks)apps/admin/src/app/(dashboard)/events/page.tsx(1 hunks)apps/admin/src/app/(dashboard)/page.tsx(1 hunks)apps/marketing/next.config.ts(1 hunks)apps/marketing/package.json(1 hunks)apps/marketing/src/app/layout.tsx(2 hunks)apps/marketing/src/components/CookieConsent/CookieConsent.tsx(1 hunks)apps/marketing/src/components/CookieConsent/index.ts(1 hunks)apps/marketing/src/env.ts(2 hunks)apps/marketing/src/instrumentation-client.ts(1 hunks)apps/web/next.config.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/src/app/layout.tsx(2 hunks)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/web/src/components/PostHogUserIdentifier/index.ts(1 hunks)apps/web/src/env.ts(2 hunks)apps/web/src/instrumentation-client.ts(1 hunks)packages/shared/src/constants.ts(1 hunks)packages/trpc/package.json(1 hunks)packages/trpc/src/env.ts(1 hunks)packages/trpc/src/lib/posthog.ts(1 hunks)packages/trpc/src/root.ts(2 hunks)packages/trpc/src/router/analytics.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- apps/admin/src/app/(dashboard)/components/StatsCard/index.ts
- apps/marketing/src/components/CookieConsent/index.ts
- apps/web/src/instrumentation-client.ts
- apps/marketing/src/app/layout.tsx
- apps/web/package.json
- packages/trpc/package.json
- apps/web/src/env.ts
- apps/marketing/next.config.ts
- apps/admin/src/app/(dashboard)/components/UserEventsTable/index.ts
- apps/marketing/package.json
- apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
- apps/marketing/src/instrumentation-client.ts
- apps/web/next.config.ts
- apps/web/src/components/PostHogUserIdentifier/index.ts
- apps/marketing/src/env.ts
- packages/trpc/src/env.ts
- apps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/web/src/app/layout.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/index.tsapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/admin/src/app/(dashboard)/page.tsxapps/admin/src/app/(dashboard)/events/[personId]/page.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsxapps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsxapps/admin/src/app/(dashboard)/events/page.tsxpackages/shared/src/constants.tspackages/trpc/src/router/analytics.tspackages/trpc/src/lib/posthog.tspackages/trpc/src/root.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/web/src/app/layout.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/index.tsapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/admin/src/app/(dashboard)/page.tsxapps/admin/src/app/(dashboard)/events/[personId]/page.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsxapps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsxapps/admin/src/app/(dashboard)/events/page.tsxpackages/shared/src/constants.tspackages/trpc/src/router/analytics.tspackages/trpc/src/lib/posthog.tspackages/trpc/src/root.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.tsapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsxapps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in apps/desktop/src/main/index.ts with override: true before any imports
Applied to files:
.env.example
🧬 Code graph analysis (9)
apps/web/src/app/layout.tsx (2)
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-24)apps/web/src/components/PostHogUserIdentifier/index.ts (1)
PostHogUserIdentifier(1-1)
apps/marketing/src/components/CookieConsent/CookieConsent.tsx (2)
apps/marketing/src/components/CookieConsent/index.ts (1)
CookieConsent(1-1)packages/shared/src/constants.ts (1)
ANALYTICS_CONSENT_KEY(23-23)
apps/admin/src/app/(dashboard)/page.tsx (3)
apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx (1)
StatsCard(18-77)apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx (1)
ActivationFunnel(25-120)apps/admin/src/app/(dashboard)/components/UserEventsTable/UserEventsTable.tsx (1)
UserEventsTable(55-161)
apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx (3)
apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts (1)
ActivationFunnel(1-1)packages/ui/src/components/ui/card.tsx (5)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)CardContent(91-91)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx (3)
apps/admin/src/app/(dashboard)/components/StatsCard/index.ts (1)
StatsCard(1-1)packages/ui/src/components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/admin/src/app/(dashboard)/events/page.tsx (5)
packages/ui/src/components/ui/card.tsx (5)
Card(85-85)CardContent(91-91)CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)packages/ui/src/components/ui/input.tsx (1)
Input(29-29)packages/ui/src/components/ui/table.tsx (5)
TableHeader(109-109)TableRow(113-113)TableHead(112-112)TableBody(110-110)TableCell(114-114)packages/ui/src/components/ui/badge.tsx (1)
Badge(46-46)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)
packages/trpc/src/router/analytics.ts (1)
packages/trpc/src/lib/posthog.ts (4)
queryPostHog(60-81)getRecentEvents(165-214)getPersonEvents(124-163)getPersons(83-122)
packages/trpc/src/lib/posthog.ts (1)
packages/trpc/src/env.ts (1)
env(4-13)
packages/trpc/src/root.ts (1)
packages/trpc/src/router/analytics.ts (1)
analyticsRouter(12-297)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 33-33: [UnorderedKey] The NEXT_PUBLIC_POSTHOG_HOST key should go before the NEXT_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
⏰ 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 Web
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (23)
apps/web/src/app/layout.tsx (2)
7-7: LGTM!The import statement is clean and follows the project's import conventions.
61-61: PostHogUserIdentifier is correctly configured as a Client Component.The component has the required
"use client"directive (line 1) and properly uses React hooks (useAuth,useUser,useEffect) with a correct dependency array. The placement inside the Providers tree before{children}is appropriate and ensures user identification occurs early in the render cycle. No action needed.apps/admin/src/app/(dashboard)/components/ActivationFunnel/index.ts (1)
1-1: LGTM!Clean barrel export following the component structure guidelines.
apps/admin/src/app/(dashboard)/events/page.tsx (1)
181-213: LGTM!Event table rendering with proper error handling and date formatting. The use of
formatDistanceToNowprovides good UX with relative timestamps.apps/admin/src/app/(dashboard)/components/ActivationFunnel/ActivationFunnel.tsx (3)
13-23: LGTM!Well-defined interfaces with appropriate optional properties for loading and error states.
30-58: LGTM!Clean early-return pattern for loading and error states with consistent Card structure.
60-119: LGTM!Solid funnel visualization with proper safeguards:
- Math.max fallback prevents division by zero
- Minimum height ensures visibility for zero-count steps
- Progressive opacity creates good visual hierarchy
- Color-coded conversion rates provide quick insights
apps/admin/src/app/(dashboard)/components/StatsCard/StatsCard.tsx (3)
7-16: LGTM!Well-structured props interface with appropriate optional fields for different use cases.
25-30: LGTM!Clear trend direction logic correctly handling positive, negative, and neutral cases.
32-76: LGTM!Excellent loading state implementation with skeleton placeholders that match the content shape. Clean conditional rendering for trend vs description.
apps/admin/src/app/(dashboard)/page.tsx (2)
12-32: LGTM!Clean data fetching setup using TRPC and React Query with proper destructuring for loading and error states.
52-104: LGTM!Well-structured dashboard composition with responsive grid layout and proper prop passing to child components. Loading and error states are handled consistently across all widgets.
apps/admin/src/app/(dashboard)/events/[personId]/page.tsx (2)
47-52: LGTM!Correct use of React 19's
use()hook for Next.js 15's async route params. This aligns with the new async Request APIs introduced in Next.js 15.
73-219: LGTM!Well-structured page with consistent UI patterns, proper loading states, and good fallback handling for missing user data. The back button navigation and card-based layout provide excellent UX.
packages/trpc/src/root.ts (2)
4-4: LGTM - Analytics router import added.The import is correctly placed with other router imports.
13-13: LGTM - Analytics router registered in root router.The analytics router is properly integrated into the application router, making all analytics procedures available under the
analyticsnamespace.apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
29-72: LGTM - Well-structured consent banner UI.The banner implementation uses proper animations, responsive layout, and clear user actions. The accessibility and UX are well thought out.
packages/trpc/src/router/analytics.ts (5)
180-209: LGTM - Event mapping logic is clean.The procedure correctly delegates to
getRecentEventsand maps the results to a typed structure. The optional person data handling is appropriate.
214-237: LGTM - User events procedure with proper pagination.The implementation correctly handles pagination via the
hasMoreflag and maps events appropriately.
242-267: LGTM - User search with proper type casting.The user search procedure correctly maps person data with safe type assertions for email and name properties.
272-296: LGTM - User statistics aggregation is correct.The query properly aggregates user counts across different time windows with safe fallback values.
123-175: SQL injection risk via string interpolation in HogQL query.Similar to
getQualityWAUTrend, theweeksvariable is directly interpolated into the query string (lines 146, 155). This poses the same injection risk.Apply the same sanitization pattern as recommended for
getQualityWAUTrend:.query(async ({ input }) => { const weeks = input?.cohortWeeks ?? 8; + if (!Number.isInteger(weeks) || weeks < 1 || weeks > 12) { + throw new Error("Invalid cohortWeeks parameter"); + }Likely an incorrect or invalid review comment.
packages/trpc/src/lib/posthog.ts (1)
1-58: LGTM - Well-structured PostHog query types.The type definitions provide comprehensive coverage of PostHog's query API with proper discriminated unions for different query kinds.
| usersData.users.map((user) => ( | ||
| <Link | ||
| key={user.id} | ||
| href={`/events/${user.id}`} | ||
| className="hover:bg-muted flex items-center gap-3 rounded-lg border p-3 transition-colors" | ||
| > | ||
| <div className="bg-muted flex h-8 w-8 items-center justify-center rounded-full"> | ||
| <LuUser className="h-4 w-4" /> | ||
| </div> | ||
| <div className="min-w-0 flex-1"> | ||
| <div className="truncate font-medium"> | ||
| {user.name || user.email || user.distinctIds[0]} | ||
| </div> | ||
| {user.email && user.name && ( | ||
| <div className="text-muted-foreground truncate text-xs"> | ||
| {user.email} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </Link> |
There was a problem hiding this comment.
Add safety check for distinctIds array access.
Line 138 accesses user.distinctIds[0] without checking if the array exists or has elements. This could cause a runtime error.
Apply this fix:
<div className="truncate font-medium">
- {user.name || user.email || user.distinctIds[0]}
+ {user.name || user.email || user.distinctIds?.[0] || 'Unknown'}
</div>🤖 Prompt for AI Agents
apps/admin/src/app/(dashboard)/events/page.tsx around lines 127-146: the JSX
renders user.distinctIds[0] without verifying distinctIds exists or has
elements; update the fallback so it only accesses the first element when the
array is present and non-empty (e.g., use optional chaining and length check or
conditional expression) and provide a safe fallback string if nothing is
available; ensure the expression short-circuits to user.name || user.email ||
(user.distinctIds?.length ? user.distinctIds[0] : 'Unknown') to prevent runtime
errors.
| const handleAccept = () => { | ||
| localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted"); | ||
| posthog.opt_in_capturing(); | ||
| setShowBanner(false); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat apps/marketing/src/instrumentation-client.tsRepository: superset-sh/superset
Length of output: 848
🏁 Script executed:
cat -n apps/marketing/src/components/CookieConsent/CookieConsent.tsxRepository: superset-sh/superset
Length of output: 2963
🏁 Script executed:
# Check if posthog is imported/used in CookieConsent
rg "posthog" apps/marketing/src/components/CookieConsent/ -A 2 -B 2Repository: superset-sh/superset
Length of output: 1038
🏁 Script executed:
# Find where instrumentation-client.ts is imported
rg "instrumentation-client" apps/marketing/src/ -B 2 -A 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the app layout/entry point
fd "layout\.(ts|tsx)$" apps/marketing/src/ -exec cat -n {} \;Repository: superset-sh/superset
Length of output: 294
🏁 Script executed:
# Check package.json to understand PostHog version
rg "posthog" apps/marketing/package.jsonRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search more broadly for instrumentation-client imports
rg "instrumentation-client"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check root layout files
find apps/marketing/src -name "layout.tsx" -o -name "layout.ts"Repository: superset-sh/superset
Length of output: 97
🏁 Script executed:
# Check the root level app structure
ls -la apps/marketing/src/ | head -20Repository: superset-sh/superset
Length of output: 477
🏁 Script executed:
cat -n apps/marketing/src/app/layout.tsxRepository: superset-sh/superset
Length of output: 2477
🏁 Script executed:
# Check if instrumentation-client is imported at the root level
rg "instrumentation" apps/marketing/Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check for Next.js instrumentation.ts/instrumentation.js at root
ls -la apps/marketing/src/instrumentation* 2>/dev/null || echo "No instrumentation file at root"Repository: superset-sh/superset
Length of output: 156
🏁 Script executed:
# Check the Next.js app directory for instrumentation
find apps/marketing -name "instrumentation.ts" -o -name "instrumentation.js"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at package.json to see if there are any build/import configurations
cat apps/marketing/package.jsonRepository: superset-sh/superset
Length of output: 1254
🏁 Script executed:
# Check if instrumentation-client.ts has a corresponding instrumentation.ts (Next.js hook)
find apps/marketing -path "*instrumentation.ts" -not -path "*/node_modules/*"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search the entire repo for imports of instrumentation-client
rg "instrumentation-client" --glob="*.tsx" --glob="*.ts" --glob="*.jsx" --glob="*.js"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's a root instrumentation file at the workspace level
ls -la instrumentation* 2>/dev/null || echo "Not found at root"Repository: superset-sh/superset
Length of output: 81
🏁 Script executed:
# Look for any references to posthog initialization
rg "posthog.init" --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for posthog.init with correct file types
rg "posthog.init" -t ts -t jsRepository: superset-sh/superset
Length of output: 235
🏁 Script executed:
# Check if instrumentation-client.ts is actually used as a Next.js instrumentation hook
# Next.js looks for instrumentation.ts/js at the app root
ls -la apps/marketing/instrumentation.* 2>/dev/null || echo "Not found"Repository: superset-sh/superset
Length of output: 73
🏁 Script executed:
# Check the actual Next.js structure - instrumentation should be at src/instrumentation.ts
ls -la apps/marketing/src/ | grep instrumentationRepository: superset-sh/superset
Length of output: 138
🏁 Script executed:
# Check next.config.js for instrumentation settings
cat apps/marketing/next.config.* 2>/dev/null || echo "No next.config found"Repository: superset-sh/superset
Length of output: 734
🏁 Script executed:
# Verify that instrumentation-client.ts is never imported
git grep -l "instrumentation-client" apps/marketing/Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's an instrumentation.ts file that might import instrumentation-client
cat apps/marketing/src/instrumentation.ts 2>/dev/null || echo "No instrumentation.ts found"Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check the CookieConsent imports more carefully to see where posthog comes from
cat -n apps/marketing/src/components/CookieConsent/CookieConsent.tsx | head -10Repository: superset-sh/superset
Length of output: 442
🏁 Script executed:
# Check if there are any client-side initialization patterns in the app
rg "useEffect|useLayoutEffect" apps/marketing/src/app/layout.tsxRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if instrumentation-client.ts is a dead file - search entire repo
rg "from.*instrumentation-client|import.*instrumentation-client"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at how posthog-js behaves when not initialized - check if it's a singleton
rg "import posthog" apps/marketing/src/ -B 2 -A 2Repository: superset-sh/superset
Length of output: 893
🏁 Script executed:
# Check if there's any lazy loading or dynamic import of instrumentation-client
rg "dynamic|lazy|import(" apps/marketing/src/app/layout.tsxRepository: superset-sh/superset
Length of output: 154
🏁 Script executed:
# Verify the actual problem: is instrumentation-client ever loaded in the marketing app?
git log --oneline -n 20 -- apps/marketing/src/instrumentation-client.tsRepository: superset-sh/superset
Length of output: 75
🏁 Script executed:
# Check how posthog-js handles method calls before initialization
# Look at web app to see if instrumentation-client is used there
rg "instrumentation" apps/web/src/app/Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's a pattern of importing instrumentation-client as a side effect
cat apps/web/src/app/layout.tsx | head -20Repository: superset-sh/superset
Length of output: 628
🏁 Script executed:
# Verify what happens when posthog methods are called uninitialized
# Check posthog-js behavior by looking at the version and any guards in the codebase
rg "posthog\." apps/marketing/src/ -B 1 -A 1Repository: superset-sh/superset
Length of output: 1268
🏁 Script executed:
# Check if instrumentation-client.ts is truly dead code by checking git history
git log --all --full-history -- apps/marketing/src/instrumentation-client.ts | head -20Repository: superset-sh/superset
Length of output: 205
🏁 Script executed:
# Verify by checking if the file was ever committed as part of imports
git show HEAD:apps/marketing/src/app/layout.tsx | grep instrumentation || echo "Not imported in current version"Repository: superset-sh/superset
Length of output: 95
🏁 Script executed:
# Check the web app to see if they properly use instrumentation-client
cat apps/web/src/instrumentation.ts 2>/dev/null || cat apps/web/src/app/instrumentation.ts 2>/dev/null || echo "Not found"Repository: superset-sh/superset
Length of output: 73
🏁 Script executed:
# Look for any root-level instrumentation.ts that might be the Next.js hook
find . -maxdepth 2 -name "instrumentation.ts" -o -name "instrumentation.js" 2>/dev/nullRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check PostHog library behavior - is there error handling for uninitialized calls?
# Look at how posthog-js is typically used
npm view posthog-js@1.306.1 descriptionRepository: superset-sh/superset
Length of output: 144
🏁 Script executed:
# Check if the marketing app layout or any other file side-effect imports instrumentation-client
rg "instrumentation-client" apps/marketing/Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Final verification: check if posthog methods gracefully handle being called before init
# by looking at the actual usage patterns in the codebase
rg "posthog\.(opt_in|opt_out|capture)" apps/marketing/src/Repository: superset-sh/superset
Length of output: 231
PostHog is called without initialization in the CookieConsent component.
The instrumentation-client.ts file that initializes PostHog is never imported or executed in the marketing app. When CookieConsent.tsx calls posthog.opt_in_capturing() at line 20, PostHog has not been initialized via posthog.init(), causing the method to run on an uninitialized client.
Either import instrumentation-client.ts at the app root (e.g., in layout.tsx or as a Next.js instrumentation.ts hook) to ensure initialization runs before the component loads, or add a guard:
const handleAccept = () => {
localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted");
- posthog.opt_in_capturing();
+ if (posthog.__loaded) {
+ posthog.opt_in_capturing();
+ }
setShowBanner(false);
};📝 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.
| const handleAccept = () => { | |
| localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted"); | |
| posthog.opt_in_capturing(); | |
| setShowBanner(false); | |
| }; | |
| const handleAccept = () => { | |
| localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted"); | |
| if (posthog.__loaded) { | |
| posthog.opt_in_capturing(); | |
| } | |
| setShowBanner(false); | |
| }; |
🤖 Prompt for AI Agents
In apps/marketing/src/components/CookieConsent/CookieConsent.tsx around lines 18
to 22, posthog.opt_in_capturing() is called even though PostHog may not have
been initialized; ensure instrumentation-client.ts that calls posthog.init()
runs before this component mounts by importing and executing it at the app root
(e.g., add import of apps/marketing/src/lib/instrumentation-client or equivalent
in layout.tsx or Next.js instrumentation hook), or add a guard in this component
that checks a shared initialized flag or posthog.hasOptedOut/initialized state
before calling opt_in_capturing() and only call it when initialization is
confirmed; update code to either perform the root import or wrap the opt-in call
with the initialization check and avoid calling posthog methods on an
uninitialized client.
| getQualityWAUTrend: adminProcedure | ||
| .input( | ||
| z.object({ | ||
| weeks: z.number().min(1).max(12).default(8), | ||
| }), | ||
| ) | ||
| .query(async ({ input }) => { | ||
| const result = await queryPostHog({ | ||
| kind: "DataVisualizationNode", | ||
| source: { | ||
| kind: "HogQLQuery", | ||
| query: ` | ||
| WITH weekly_active AS ( | ||
| SELECT | ||
| person_id, | ||
| toStartOfWeek(timestamp) as week, | ||
| count(DISTINCT toDate(timestamp)) as active_days | ||
| FROM events | ||
| WHERE timestamp >= now() - INTERVAL ${input.weeks} WEEK | ||
| AND event IN ('task_started', 'task_completed') | ||
| GROUP BY person_id, week | ||
| HAVING active_days >= 3 | ||
| ) | ||
| SELECT | ||
| week, | ||
| count(DISTINCT person_id) as quality_wau | ||
| FROM weekly_active | ||
| GROUP BY week | ||
| ORDER BY week | ||
| `, | ||
| }, | ||
| }); | ||
|
|
||
| return { | ||
| data: result.results as Array<[string, number]>, | ||
| weeks: input.weeks, | ||
| }; | ||
| }), |
There was a problem hiding this comment.
SQL injection risk via string interpolation in HogQL query.
input.weeks is directly interpolated into the query string (lines 61, 62). While Zod validation constrains the input, string interpolation in SQL-like queries is inherently unsafe. If validation is bypassed or the constraint changes, this could lead to injection.
PostHog's query API should support parameterized queries. If not available, add explicit type assertion and sanitization:
.query(async ({ input }) => {
+ const weeks = Number.parseInt(String(input.weeks), 10);
+ if (!Number.isInteger(weeks) || weeks < 1 || weeks > 12) {
+ throw new Error("Invalid weeks parameter");
+ }
+
const result = await queryPostHog({
kind: "DataVisualizationNode",
source: {
kind: "HogQLQuery",
query: `
WITH weekly_active AS (
SELECT
person_id,
toStartOfWeek(timestamp) as week,
count(DISTINCT toDate(timestamp)) as active_days
FROM events
- WHERE timestamp >= now() - INTERVAL ${input.weeks} WEEK
+ WHERE timestamp >= now() - INTERVAL ${weeks} WEEK
AND event IN ('task_started', 'task_completed')
GROUP BY person_id, week
HAVING active_days >= 3
)📝 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.
| getQualityWAUTrend: adminProcedure | |
| .input( | |
| z.object({ | |
| weeks: z.number().min(1).max(12).default(8), | |
| }), | |
| ) | |
| .query(async ({ input }) => { | |
| const result = await queryPostHog({ | |
| kind: "DataVisualizationNode", | |
| source: { | |
| kind: "HogQLQuery", | |
| query: ` | |
| WITH weekly_active AS ( | |
| SELECT | |
| person_id, | |
| toStartOfWeek(timestamp) as week, | |
| count(DISTINCT toDate(timestamp)) as active_days | |
| FROM events | |
| WHERE timestamp >= now() - INTERVAL ${input.weeks} WEEK | |
| AND event IN ('task_started', 'task_completed') | |
| GROUP BY person_id, week | |
| HAVING active_days >= 3 | |
| ) | |
| SELECT | |
| week, | |
| count(DISTINCT person_id) as quality_wau | |
| FROM weekly_active | |
| GROUP BY week | |
| ORDER BY week | |
| `, | |
| }, | |
| }); | |
| return { | |
| data: result.results as Array<[string, number]>, | |
| weeks: input.weeks, | |
| }; | |
| }), | |
| getQualityWAUTrend: adminProcedure | |
| .input( | |
| z.object({ | |
| weeks: z.number().min(1).max(12).default(8), | |
| }), | |
| ) | |
| .query(async ({ input }) => { | |
| const weeks = Number.parseInt(String(input.weeks), 10); | |
| if (!Number.isInteger(weeks) || weeks < 1 || weeks > 12) { | |
| throw new Error("Invalid weeks parameter"); | |
| } | |
| const result = await queryPostHog({ | |
| kind: "DataVisualizationNode", | |
| source: { | |
| kind: "HogQLQuery", | |
| query: ` | |
| WITH weekly_active AS ( | |
| SELECT | |
| person_id, | |
| toStartOfWeek(timestamp) as week, | |
| count(DISTINCT toDate(timestamp)) as active_days | |
| FROM events | |
| WHERE timestamp >= now() - INTERVAL ${weeks} WEEK | |
| AND event IN ('task_started', 'task_completed') | |
| GROUP BY person_id, week | |
| HAVING active_days >= 3 | |
| ) | |
| SELECT | |
| week, | |
| count(DISTINCT person_id) as quality_wau | |
| FROM weekly_active | |
| GROUP BY week | |
| ORDER BY week | |
| `, | |
| }, | |
| }); | |
| return { | |
| data: result.results as Array<[string, number]>, | |
| weeks: input.weeks, | |
| }; | |
| }), |
🤖 Prompt for AI Agents
In packages/trpc/src/router/analytics.ts around lines 43 to 80, the HogQL query
currently interpolates input.weeks directly into the query string which creates
a SQL-injection risk; update the call to either use PostHog/HogQL parameterized
queries if supported (pass weeks as a bound parameter instead of string
interpolation) or otherwise coerce and validate the value before interpolation
by converting input.weeks to a safe integer (e.g., Math.floor/Number, clamp to
the allowed range) and explicitly serialize it as a numeric literal so only a
plain integer can be inserted into the query string.
1 participant
Summary
/ingestproxy to bypass ad blockersANALYTICS_CONSENT_KEYconstant for cross-subdomain persistenceChanges
apps/web/src/instrumentation-client.ts- PostHog initapps/web/src/components/PostHogUserIdentifier/- Syncs Clerk user to PostHogapps/marketing/src/instrumentation-client.ts- PostHog init with GDPR opt-outapps/marketing/src/components/CookieConsent/- Cookie consent bannerpackages/shared/src/constants.ts- AddedANALYTICS_CONSENT_KEYenv.tsfiles with PostHog env varsnext.config.tsfiles with/ingestrewritesTest plan
NEXT_PUBLIC_POSTHOG_KEYto .env🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.