Integrate PostHog#254
Conversation
Add PostHog provider with auto pageview tracking (via history_change), user identification on auth state changes, error boundary, and exception autocapture. Gracefully no-ops when VITE_POSTHOG_KEY is absent.
Track RPC duration, slow queries (>20s), errors, and timeouts for the get_snippets endpoint. Add proper abort signal handling to cancel in-flight requests when queries change.
WalkthroughThis pull request integrates PostHog analytics into the application by adding environment variable configuration, dependencies, and instrumentation. It wraps the app with PostHogProvider, instruments snippet fetching with timing and error tracking, and ties analytics identification to the authenticated user. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate 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 |
✅ Preview Deployment Ready!URL: https://pr-254-verdad-frontend.fly.dev Preview auto-updates on push. Destroyed when PR closes. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces PostHog, a powerful analytics and telemetry platform, to the application. This integration will provide valuable insights into how users interact with the application, track key performance indicators, and automatically report exceptions. These changes are crucial for understanding user behavior, optimizing performance, and proactively addressing issues, ultimately leading to a more stable and user-friendly experience. Additionally, the snippet fetching mechanism has been refined to include cancellation capabilities and improved error handling. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/apis/snippet.ts`:
- Around line 52-59: The telemetry code currently spreads getSnippetsOptions
(which contains p_search_term and p_filter) into analytics causing raw user
input to be sent; update places that spread getSnippetsOptions (the telemetry
payload usage around where telemetry/event tracking is called) to instead build
a sanitized payload that omits p_search_term and p_filter and includes only
non-sensitive derived fields (e.g. has_search: Boolean(searchTerm),
filters_count: Object.keys(actualFilters || {}).length or similar, language,
orderBy, page, page_size). Replace the direct spread of getSnippetsOptions with
this explicit sanitized object wherever getSnippetsOptions was used for
telemetry (preserving the original getSnippetsOptions for API calls).
In `@src/providers/posthog.tsx`:
- Around line 24-33: The current useEffect calls posthog.reset() whenever userId
is falsy, which resets distinct IDs on initial mount and fragments anonymous
activity; change the effect to only call posthog.reset() when the user
transitions from authenticated to unauthenticated (a real logout). Implement a
previous-userId check (e.g., a useRef prevUserId) inside the useEffect that
compares prevUserId to userId and only calls posthog.reset() if prevUserId was
truthy and userId is now falsy; continue to call posthog.identify(userId, {
email: userEmail }) when userId becomes truthy, and update prevUserId after
handling. Ensure the effect still depends on posthog, userId, and userEmail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dce0365-a48e-46f5-88ae-b4630528b8c9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.env.production.examplepackage.jsonsrc/App.tsxsrc/apis/snippet.tssrc/hooks/useSnippets.tsxsrc/providers/posthog.tsxsrc/types/snippet.tssrc/vite-env.d.ts
💤 Files with no reviewable changes (1)
- src/types/snippet.ts
| const getSnippetsOptions = { | ||
| page: pageParam, | ||
| page_size: pageSize, | ||
| p_language: language, | ||
| p_filter: actualFilters, | ||
| p_order_by: orderBy, | ||
| p_search_term: searchTerm | ||
| }) | ||
| } |
There was a problem hiding this comment.
Don't send raw search input and filters to PostHog.
Lines 73-77 and 83-89 spread getSnippetsOptions into telemetry, which includes p_search_term and p_filter. That exports raw user queries/filter state to analytics and creates very high-cardinality events. Whitelist derived, non-sensitive fields instead.
Safer payload shape
const getSnippetsOptions = {
page: pageParam,
page_size: pageSize,
p_language: language,
p_filter: actualFilters,
p_order_by: orderBy,
p_search_term: searchTerm
}
+ const analyticsProps = {
+ page: pageParam,
+ page_size: pageSize,
+ p_language: language,
+ p_order_by: orderBy,
+ has_search_term: searchTerm.trim().length > 0,
+ filter_count: Object.keys(actualFilters ?? {}).length
+ }
@@
const isTimeout = /timeout|canceling statement/i.test(error.message)
posthog.captureException(error, {
- ...getSnippetsOptions,
+ ...analyticsProps,
duration_ms: durationMs,
is_timeout: isTimeout
})
@@
posthog.capture('get_snippets_rpc', {
- ...getSnippetsOptions,
+ ...analyticsProps,
duration_ms: durationMs,
status: durationMs > SLOW_THRESHOLD_MS ? 'warning' : 'success',
total_pages: data.total_pages,
total_snippets: data.num_of_snippets
})Also applies to: 73-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/apis/snippet.ts` around lines 52 - 59, The telemetry code currently
spreads getSnippetsOptions (which contains p_search_term and p_filter) into
analytics causing raw user input to be sent; update places that spread
getSnippetsOptions (the telemetry payload usage around where telemetry/event
tracking is called) to instead build a sanitized payload that omits
p_search_term and p_filter and includes only non-sensitive derived fields (e.g.
has_search: Boolean(searchTerm), filters_count: Object.keys(actualFilters ||
{}).length or similar, language, orderBy, page, page_size). Replace the direct
spread of getSnippetsOptions with this explicit sanitized object wherever
getSnippetsOptions was used for telemetry (preserving the original
getSnippetsOptions for API calls).
| useEffect(() => { | ||
| if (!userId) { | ||
| posthog.reset() | ||
| return | ||
| } | ||
|
|
||
| posthog.identify(userId, { | ||
| email: userEmail | ||
| }) | ||
| }, [posthog, userId, userEmail]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'useState|onAuthStateChange|getSession|setUser|signOut|logout' src/providers/auth.tsx
sed -n '1,260p' src/providers/auth.tsxRepository: PublicDataWorks/verdad-frontend
Length of output: 6470
Avoid resetting PostHog on every anonymous render.
Lines 25–27 reset PostHog whenever userId is falsy, including on initial mount before session restoration completes. This rotates the distinct ID before authentication finishes, causing anonymous activity to fragment instead of merging cleanly into the later identified user. Reset only when transitioning from a logged-in state to logged-out (i.e., a real logout), not on initial mount or anonymous states.
Suggested direction
-import { useEffect } from 'react'
+import { useEffect, useRef } from 'react'
@@
function PostHogTracking({ children }: PostHogProviderProps) {
const posthog = usePostHog()
const { user } = useAuth()
const userId = user?.id
const userEmail = user?.email
+ const previousUserId = useRef<string | undefined>(undefined)
useEffect(() => {
if (!userId) {
- posthog.reset()
+ if (previousUserId.current) {
+ posthog.reset()
+ previousUserId.current = undefined
+ }
return
}
+ previousUserId.current = userId
posthog.identify(userId, {
email: userEmail
})
}, [posthog, userId, userEmail])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/providers/posthog.tsx` around lines 24 - 33, The current useEffect calls
posthog.reset() whenever userId is falsy, which resets distinct IDs on initial
mount and fragments anonymous activity; change the effect to only call
posthog.reset() when the user transitions from authenticated to unauthenticated
(a real logout). Implement a previous-userId check (e.g., a useRef prevUserId)
inside the useEffect that compares prevUserId to userId and only calls
posthog.reset() if prevUserId was truthy and userId is now falsy; continue to
call posthog.identify(userId, { email: userEmail }) when userId becomes truthy,
and update prevUserId after handling. Ensure the effect still depends on
posthog, userId, and userEmail.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates PostHog for analytics and error tracking. The implementation is well-structured, introducing a PostHogProvider and instrumenting the fetchSnippets API call for performance monitoring and error reporting. The changes also correctly add support for request cancellation using AbortSignal. I've left a few comments on making the error handling logic more robust and cleaning up the PostHog configuration.
Include VITE_POSTHOG_KEY and VITE_POSTHOG_HOST as build arguments to support PostHog analytics integration during the Docker build process.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 11-12: The Dockerfile now declares ARG VITE_POSTHOG_KEY and ARG
VITE_POSTHOG_HOST but the Fly deployment workflows don't pass them through;
update the Fly deploy steps (the flyctl build/deploy commands in your CI
workflows) to include --build-arg VITE_POSTHOG_KEY=${{ secrets.VITE_POSTHOG_KEY
}} and --build-arg VITE_POSTHOG_HOST=${{ secrets.VITE_POSTHOG_HOST }} (or the
appropriate env/secret variables used in the repo) so the build-time args are
populated and the PostHog integration is compiled with real values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Add VITE_POSTHOG_KEY and VITE_POSTHOG_HOST build arguments to both the production deployment and PR preview workflows to ensure PostHog analytics configuration is properly passed during Docker builds.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/fly-pr-preview.yml:
- Around line 100-102: The workflow is injecting production PostHog credentials
into PR previews (VITE_POSTHOG_KEY and VITE_POSTHOG_HOST) which will send
reviewer traffic/PII to production; remove those two --build-arg lines from the
fly-pr-preview workflow (or set them to empty) so previews do not receive the
production key, and add a defensive guard in src/providers/posthog.tsx (e.g.,
check a new env like VITE_ENABLE_POSTHOG or require NODE_ENV==='production'
before calling the PostHog init/identify code) so PostHog only initializes when
an explicit preview project or production flag is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: afe6b324-b62a-4c63-babf-10528699cd5b
📒 Files selected for processing (2)
.github/workflows/deploy.yml.github/workflows/fly-pr-preview.yml
Add a thin wrapper (src/lib/posthog.ts) that no-ops capture and captureException when VITE_POSTHOG_KEY is not set, preventing calls to an uninitialized posthog-js singleton.
PostHog environment variables are not needed for PR preview deployments. Removing unnecessary build arguments to simplify the workflow configuration.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/apis/snippet.ts (1)
52-59:⚠️ Potential issue | 🟠 MajorWhitelist analytics props instead of spreading RPC params.
getSnippetsOptionsstill containsp_search_termandp_filter, and both telemetry paths spread it straight into PostHog. That sends raw user input/high-cardinality filter state to analytics; build a sanitizedanalyticsPropsobject and use that for both calls.Also applies to: 73-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/snippet.ts` around lines 52 - 59, getSnippetsOptions currently includes raw user inputs (p_search_term, p_filter) which are being spread into analytics; create a sanitized analyticsProps object that whitelists only safe fields (e.g., page, page_size, p_language, p_order_by, plus low-cardinality/sanitized filter tokens) and omit p_search_term and any high-cardinality filter values, then use analyticsProps for PostHog/event tracking instead of spreading getSnippetsOptions or RPC params; update both the getSnippetsOptions construction and the places referenced around lines 73-89 to send analyticsProps to PostHog while keeping getSnippetsOptions intact for the RPC call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/apis/snippet.ts`:
- Around line 67-78: The current error handling logs every error before checking
for aborts; move the console.error call inside the non-aborted branch so
cancelled requests aren't logged as errors. Specifically, evaluate isAborted
using the existing /AbortError/i test on error.message, then only call
console.error and captureException when !isAborted (use the same
getSnippetsOptions and durationMs context and preserve the is_timeout check).
Ensure you still guard usage of error.message if it can be undefined before
running the regex.
In `@src/lib/posthog.ts`:
- Around line 5-16: The wrappers capture and captureException currently mix bare
"return" with "return posthog..." which violates consistent-return; make them
explicit side-effect-only by removing returned values: change both functions to
have a void return type, and when isEnabled is true call posthog.capture(...)
and posthog.captureException(...) as standalone statements (no returned value)
while keeping the early "if (!isEnabled) return" guard; reference the functions
capture, captureException and the delegated
posthog.capture/posthog.captureException and the isEnabled flag when applying
the change.
---
Duplicate comments:
In `@src/apis/snippet.ts`:
- Around line 52-59: getSnippetsOptions currently includes raw user inputs
(p_search_term, p_filter) which are being spread into analytics; create a
sanitized analyticsProps object that whitelists only safe fields (e.g., page,
page_size, p_language, p_order_by, plus low-cardinality/sanitized filter tokens)
and omit p_search_term and any high-cardinality filter values, then use
analyticsProps for PostHog/event tracking instead of spreading
getSnippetsOptions or RPC params; update both the getSnippetsOptions
construction and the places referenced around lines 73-89 to send analyticsProps
to PostHog while keeping getSnippetsOptions intact for the RPC call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8663efac-b0ba-4630-8a6d-c4853e45f7b4
📒 Files selected for processing (2)
src/apis/snippet.tssrc/lib/posthog.ts
| export function capture( | ||
| ...args: Parameters<typeof posthog.capture> | ||
| ): ReturnType<typeof posthog.capture> | undefined { | ||
| if (!isEnabled) return | ||
| return posthog.capture(...args) | ||
| } | ||
|
|
||
| export function captureException( | ||
| ...args: Parameters<typeof posthog.captureException> | ||
| ): ReturnType<typeof posthog.captureException> | undefined { | ||
| if (!isEnabled) return | ||
| return posthog.captureException(...args) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and read the relevant section
cat -n src/lib/posthog.tsRepository: PublicDataWorks/verdad-frontend
Length of output: 686
🏁 Script executed:
# Check the callers mentioned in src/apis/snippet.ts
cat -n src/apis/snippet.ts | sed -n '70,95p'Repository: PublicDataWorks/verdad-frontend
Length of output: 954
🏁 Script executed:
# Search for all usages of the capture and captureException functions
rg -n "capture\(" src/ --type ts --type tsx -B 2 -A 2Repository: PublicDataWorks/verdad-frontend
Length of output: 102
🏁 Script executed:
# Also check for captureException usages
rg -n "captureException\(" src/ --type ts --type tsx -B 2 -A 2Repository: PublicDataWorks/verdad-frontend
Length of output: 102
🏁 Script executed:
# Search for all capture( and captureException( usages
rg -n "capture\(" src/ -B 2 -A 2Repository: PublicDataWorks/verdad-frontend
Length of output: 715
🏁 Script executed:
# Check for any linting configuration
fd -e "eslintrc*" -e "eslint.config.*" -e ".prettierrc*"Repository: PublicDataWorks/verdad-frontend
Length of output: 57
🏁 Script executed:
# Also check package.json for linting rules
cat package.json | grep -A 20 '"eslint\|"lint'Repository: PublicDataWorks/verdad-frontend
Length of output: 1899
🏁 Script executed:
# Search for captureException usages more thoroughly
rg -n "captureException" src/Repository: PublicDataWorks/verdad-frontend
Length of output: 490
🏁 Script executed:
# Check if there's an eslintrc config file
find . -maxdepth 3 -name ".eslintrc*" -o -name "eslint.config.*"Repository: PublicDataWorks/verdad-frontend
Length of output: 115
🏁 Script executed:
# Look for the actual linting rules configuration
cat .eslintrc.cjs 2>/dev/null || cat .eslintrc.json 2>/dev/null || cat .eslintrc.js 2>/dev/null || echo "No eslintrc found in root"Repository: PublicDataWorks/verdad-frontend
Length of output: 5618
🏁 Script executed:
# Double-check if these functions are imported/exported elsewhere
rg -n "from.*posthog|import.*capture" src/ | grep -v node_modulesRepository: PublicDataWorks/verdad-frontend
Length of output: 483
Make these wrappers explicitly side-effect-only.
Both functions mix bare return with return posthog..., triggering the consistent-return linting rule. The only callers in src/apis/snippet.ts (lines 73 and 83) treat both helpers as fire-and-forget, so returning the delegated value serves no purpose.
Suggested fix
export function capture(
...args: Parameters<typeof posthog.capture>
-): ReturnType<typeof posthog.capture> | undefined {
+): void {
if (!isEnabled) return
- return posthog.capture(...args)
+ posthog.capture(...args)
}
export function captureException(
...args: Parameters<typeof posthog.captureException>
-): ReturnType<typeof posthog.captureException> | undefined {
+): void {
if (!isEnabled) return
- return posthog.captureException(...args)
+ posthog.captureException(...args)
}📝 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 function capture( | |
| ...args: Parameters<typeof posthog.capture> | |
| ): ReturnType<typeof posthog.capture> | undefined { | |
| if (!isEnabled) return | |
| return posthog.capture(...args) | |
| } | |
| export function captureException( | |
| ...args: Parameters<typeof posthog.captureException> | |
| ): ReturnType<typeof posthog.captureException> | undefined { | |
| if (!isEnabled) return | |
| return posthog.captureException(...args) | |
| export function capture( | |
| ...args: Parameters<typeof posthog.capture> | |
| ): void { | |
| if (!isEnabled) return | |
| posthog.capture(...args) | |
| } | |
| export function captureException( | |
| ...args: Parameters<typeof posthog.captureException> | |
| ): void { | |
| if (!isEnabled) return | |
| posthog.captureException(...args) | |
| } |
🧰 Tools
🪛 ESLint
[error] 9-9: Function 'capture' expected no return value.
(consistent-return)
[error] 16-16: Function 'captureException' expected no return value.
(consistent-return)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/posthog.ts` around lines 5 - 16, The wrappers capture and
captureException currently mix bare "return" with "return posthog..." which
violates consistent-return; make them explicit side-effect-only by removing
returned values: change both functions to have a void return type, and when
isEnabled is true call posthog.capture(...) and posthog.captureException(...) as
standalone statements (no returned value) while keeping the early "if
(!isEnabled) return" guard; reference the functions capture, captureException
and the delegated posthog.capture/posthog.captureException and the isEnabled
flag when applying the change.
1 participant
Summary by CodeRabbit