Add user funnel analytics tracking#838
Open
JoaquinBN wants to merge 4 commits into
Open
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds a shared analytics helper and instruments route guards, navigation/auth entry points, onboarding and submission views, and builder/community/validator journeys with page, click, intent, timing, and outcome tracking. It also adds tests for the analytics module. ChangesFrontend analytics instrumentation
Sequence Diagram(s)sequenceDiagram
participant RouteGuard as App.svelte
participant EntryPoint as Navbar/Sidebar/SocialLink/SubmitContribution
participant Analytics as frontend/src/lib/analytics.js
participant AuthButton as AuthButton.svelte
participant Journey as Builder/Community/Validator routes
RouteGuard->>Analytics: trackEvent('protected_route_redirect'/'role_locked_redirect', ...)
RouteGuard->>Analytics: trackPageView($location, getAnalyticsContext())
EntryPoint->>Analytics: setConnectWalletIntent(...)
EntryPoint->>Analytics: trackEvent(...)
AuthButton->>Analytics: consumeConnectWalletIntent()
AuthButton->>Analytics: trackEvent('wallet_provider_selected'/'wallet_auth_started'/'wallet_auth_success'/'wallet_auth_failed', ...)
Journey->>Analytics: trackEvent('journey_view'/'journey_step_visible'/'journey_step_action_click', ...)
Journey->>Analytics: markFunnelTime()/markLifecycleTime()
Journey->>Analytics: trackEvent('journey_started'/'journey_start_error'/'journey_exit'/'journey_completed', ...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/ProfileCompletionGuard.svelte`:
- Line 70: The `ProfileCompletionGuard` analytics payload is sending `$location`
directly as `source_route`, which can leak wallet-address segments into
analytics. Update the `ProfileCompletionGuard.svelte` flow to pass
`source_route` through `templateRoute` from `lib/analytics.js` before emission,
and make sure the import is present where the analytics event is built.
In `@frontend/src/components/social-tasks/SocialTaskCard.svelte`:
- Around line 116-119: The Verify flow in SocialTaskCard.svelte is
double-counting journey_step_action_click because callComplete defaults
trackAction to true and the Verify button path is not suppressing it. Update the
Verify button handler to call callComplete with trackAction set to false,
matching the click-through path and preventing the duplicate event emission from
handleOpened/callComplete.
In `@frontend/src/lib/analytics.js`:
- Around line 44-46: URL_RE in analytics sanitization is too narrow because it
only matches strings that start with http(s), so embedded links inside free-text
values can still be sent. Update the value-checking logic in analytics.js to use
a separate unanchored URL detector for string values, and apply it in the same
sanitization flow that already uses ADDRESS_RE and EMAIL_RE so embedded URLs are
blocked before truncation.
- Around line 167-173: The shouldDropKey helper currently only blocks keys
ending in _id, so bare sensitive identifiers like id, slug, and token can still
be sent to analytics. Update shouldDropKey in analytics.js to explicitly reject
these bare identifier keys, either by extending FORBIDDEN_KEYS or by switching
the helper to a strict allowlist, and keep the existing checks around
SAFE_ID_KEYS, error_message, and raw intact.
- Line 96: The route label mapping in analytics is still emitting “badge” for
legacy matches, so update the template alias in analytics.js to use the current
product term instead. In the route-label mapping near the badge-style regex
entry, keep matching the legacy path if needed but change the emitted analytics
label to the contribution term (for example, use the contribution equivalent of
the current template) so reports no longer contain “badge”.
In `@frontend/src/routes/BuilderJourney.svelte`:
- Around line 479-483: The `error_code` branch in `BuilderJourney.svelte` is
unreachable because the `if (error?.code !== 4001)` guard excludes user
rejections, so `journey_step_error` never records `user_rejected`. Update the
`wallet_add_chain` error handling around `trackBuilderStepEvent` so rejection
analytics are emitted outside or before the toast guard, and keep the
conditional only for the toast/display logic. Use the `error?.code` check and
`trackBuilderStepEvent('journey_step_error', 'networks', ...)` block as the
anchor when refactoring.
In `@frontend/src/tests/analytics.test.js`:
- Around line 198-205: The analytics test for getAnalyticsContext currently only
checks a partial shape with toMatchObject, so it would miss accidental exposure
of identifier fields. Update the assertion around getAnalyticsContext() to keep
the existing expected context checks and add negative assertions that user,
email, and address are not present in the returned object.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c34c85a8-8cbb-4194-9209-def6fad9ea96
📒 Files selected for processing (17)
frontend/src/App.sveltefrontend/src/components/AuthButton.sveltefrontend/src/components/Navbar.sveltefrontend/src/components/ProfileCompletionGuard.sveltefrontend/src/components/Sidebar.sveltefrontend/src/components/SocialLink.sveltefrontend/src/components/funnel/RoleFunnel.sveltefrontend/src/components/funnel/RoleLanding.sveltefrontend/src/components/portal/submit-contribution/SubmitContribution.sveltefrontend/src/components/social-tasks/SocialTaskCard.sveltefrontend/src/lib/analytics.jsfrontend/src/routes/BuilderJourney.sveltefrontend/src/routes/CommunityJourney.sveltefrontend/src/routes/MySubmissions.sveltefrontend/src/routes/SubmitContribution.sveltefrontend/src/routes/ValidatorWaitlist.sveltefrontend/src/tests/analytics.test.js
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/routes/BuilderJourney.svelte (1)
238-265: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winWait for user hydration before auto-starting the builder journey.
When
useris stillundefinedon first mount,!user?.has_builder_welcome && !user?.builderevaluates to true, sostartBuilderJourney()can fire too early. That can create falsejourney_started/journey_start_erroranalytics and an unnecessary backend call. Move this into a one-shot effect that waits for$userStore.loading === falseand a realuserobject.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/BuilderJourney.svelte` around lines 238 - 265, The auto-start logic in BuilderJourney should not run until user hydration is complete, because the current `!user?.has_builder_welcome && !user?.builder` check can fire while `user` is still undefined. Move the `startBuilderJourney()` call into a one-time effect tied to `userStore.loading === false` and a resolved `user` object, and guard it so it only runs once after hydration; keep the existing analytics (`journey_started`, `journey_start_error`) and warning handling inside that hydrated flow.frontend/src/components/ProfileCompletionGuard.svelte (2)
162-188: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTrim the profile fields before validating and submitting.
if (!email || !name)treats whitespace-only input as valid, but this guard already considers trimmed-empty values incomplete. A" "display name can get submitted and drop the user straight back into the completion gate.Suggested fix
async function handleProfileSubmit() { + const trimmedEmail = email.trim(); + const trimmedName = name.trim(); + trackEvent('profile_completion_submit', getAnalyticsContext({ selected_role: selectedRole, preselected_role: preselectedRole, })); // Validate inputs - if (!email || !name) { + if (!trimmedEmail || !trimmedName) { trackEvent('profile_completion_error', getAnalyticsContext({ selected_role: selectedRole, error_stage: 'validation', })); profileError = 'Please provide both email and display name'; return; } - if (!isValidEmail(email)) { + if (!isValidEmail(trimmedEmail)) { trackEvent('profile_completion_error', getAnalyticsContext({ selected_role: selectedRole, error_stage: 'validation', })); profileError = 'Please enter a valid email address'; return; } @@ - if (email) updateData.email = email; - if (name) updateData.name = name; + updateData.email = trimmedEmail; + updateData.name = trimmedName;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ProfileCompletionGuard.svelte` around lines 162 - 188, Trim the profile inputs in ProfileCompletionGuard.svelte before running the validation and building the update payload so whitespace-only values are treated as empty. Update the logic around the validation block that checks email and name to use trimmed email/name values for the !email || !name and isValidEmail checks, and use those trimmed values when populating updateData before submit. Keep the existing analytics tracking and error messages, but make sure the submit flow cannot send a display name consisting only of spaces.
163-177: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRoute submit failures through the toast system too.
These validation/backend branches only set
profileError, so this flow bypasses the shared notification channel required for components and routes. Keep the inline message if you want, but also emit a toast here. As per coding guidelines,frontend/**/{components,routes}/**/*.{ts,tsx,js,svelte}: Always handle API errors with try-catch blocks and provide user-facing error messages via toast notifications.Also applies to: 215-233
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ProfileCompletionGuard.svelte` around lines 163 - 177, The validation and backend failure paths in ProfileCompletionGuard currently only set profileError, so they bypass the shared toast notification flow. Update the submit/error handling in the profile completion logic to also emit a user-facing toast alongside the inline message, including the existing validation branches and the backend-related handling later in the same flow. Use the existing trackEvent/profileError flow as the anchor, and make sure the route/component error handling is wrapped with try-catch where applicable so failures are surfaced through the toast system.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/src/components/ProfileCompletionGuard.svelte`:
- Around line 162-188: Trim the profile inputs in ProfileCompletionGuard.svelte
before running the validation and building the update payload so whitespace-only
values are treated as empty. Update the logic around the validation block that
checks email and name to use trimmed email/name values for the !email || !name
and isValidEmail checks, and use those trimmed values when populating updateData
before submit. Keep the existing analytics tracking and error messages, but make
sure the submit flow cannot send a display name consisting only of spaces.
- Around line 163-177: The validation and backend failure paths in
ProfileCompletionGuard currently only set profileError, so they bypass the
shared toast notification flow. Update the submit/error handling in the profile
completion logic to also emit a user-facing toast alongside the inline message,
including the existing validation branches and the backend-related handling
later in the same flow. Use the existing trackEvent/profileError flow as the
anchor, and make sure the route/component error handling is wrapped with
try-catch where applicable so failures are surfaced through the toast system.
In `@frontend/src/routes/BuilderJourney.svelte`:
- Around line 238-265: The auto-start logic in BuilderJourney should not run
until user hydration is complete, because the current
`!user?.has_builder_welcome && !user?.builder` check can fire while `user` is
still undefined. Move the `startBuilderJourney()` call into a one-time effect
tied to `userStore.loading === false` and a resolved `user` object, and guard it
so it only runs once after hydration; keep the existing analytics
(`journey_started`, `journey_start_error`) and warning handling inside that
hydrated flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: af524d57-88ad-4324-adf8-cb0433dcce98
📒 Files selected for processing (5)
frontend/src/components/ProfileCompletionGuard.sveltefrontend/src/components/social-tasks/SocialTaskCard.sveltefrontend/src/lib/analytics.jsfrontend/src/routes/BuilderJourney.sveltefrontend/src/tests/analytics.test.js
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/src/components/Sidebar.svelte (1)
269-269: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep these as plain in-app anchors.
These link updates call
preventDefault()and route throughnavigate(...); for normal in-app navigation, leave the<a href="...">behavior intact and track clicks without taking over navigation.As per coding guidelines, “Use plain
<a href="/path">anchors for in-app navigation (global interceptor handles SPA routing) instead of callingpush()in click handlers.”Also applies to: 493-493, 734-734, 923-927
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Sidebar.svelte` at line 269, The Sidebar.svelte link handlers are overriding normal anchor behavior by calling preventDefault() and navigate(...), which should be removed for in-app navigation. Update the affected anchor elements in the Sidebar component to use plain <a href="..."> links and keep any click tracking separate from routing, so the global interceptor can handle SPA navigation; apply this to the leaderboard and the other matching links referenced in the Sidebar markup.Source: Coding guidelines
frontend/src/routes/ValidatorWaitlist.svelte (1)
187-193: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the caught error when classifying start failures.
This always reports
journey_start_error.error_stageasbackend; network failures lose their classification.🐛 Proposed fix
- } catch (_) { + } catch (err) { trackEvent('journey_start_error', getAnalyticsContext({ role_context: 'validator', selected_role: 'validator', surface: 'journey', - error_stage: 'backend', + error_stage: err.response?.status ? 'backend' : 'network', }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/ValidatorWaitlist.svelte` around lines 187 - 193, The start-failure tracking in ValidatorWaitlist.svelte is discarding the caught exception, so every journey_start_error is classified as backend and network failures are misreported. Update the catch block around the journey start flow to retain the caught error and derive error_stage from it before calling trackEvent/getAnalyticsContext, using the existing journey_start_error reporting path and the validator role_context/selected_role fields so the classification reflects the actual failure type.frontend/src/routes/CommunityJourney.svelte (1)
425-456: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecompute lifecycle durations after marking role unlock.
claimParamsis created beforemarkLifecycleTime('role_unlocked:community'), so the success/completed/unlocked analytics can miss the just-recorded unlock timing.🐛 Proposed fix
if (res.data?.user) userStore.updateUser(res.data.user); await userStore.loadUser?.(); markLifecycleTime('role_unlocked:community'); - trackEvent('community_role_claim_success', getAnalyticsContext(claimParams)); + const successParams = { + ...claimParams, + ...getLifecycleDurations('community'), + }; + trackEvent('community_role_claim_success', getAnalyticsContext(successParams)); trackEvent('journey_completed', getAnalyticsContext({ - ...claimParams, + ...successParams, journey_state: 'completed', })); trackEvent('role_unlocked', getAnalyticsContext({ - ...claimParams, + ...successParams, role_context: 'community', selected_role: 'community',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/CommunityJourney.svelte` around lines 425 - 456, The analytics payload in completeJourney() is built too early, so the success/completed/unlocked events can omit the newly recorded role unlock timing. Move the lifecycle timing update and then recompute the claim analytics context after markLifecycleTime('role_unlocked:community') in CommunityJourney.svelte, or build a fresh payload for the post-unlock trackEvent calls so getLifecycleDurations('community') reflects the latest state.frontend/src/routes/BuilderJourney.svelte (2)
456-484: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmit
journey_step_verifiedfor individual network adds.Bulk add reports
networksas verified, but adding Bradbury/Asimov/Studio one-by-one never emits the verified event when the final missing network is added.🐛 Proposed fix
async function addNetwork(network, kind, { silent = false } = {}) { if (typeof window === 'undefined') return; const walletProvider = provider || window.ethereum; if (!walletProvider) { showWarning('Please connect your wallet first.'); return false; } + const networksCompleteBefore = NETWORKS.every((item) => networkDone(item.kind)); if (kind === 'bradbury') isAddingBradbury = true; if (kind === 'asimov') isAddingAsimov = true; if (kind === 'studio') isAddingStudio = true; @@ }); markNetworkAdded(kind); + if (!silent && !networksCompleteBefore && NETWORKS.every((item) => networkDone(item.kind))) { + trackBuilderStepEvent('journey_step_verified', 'networks'); + } if (!silent) showSuccess(`${network.chainName} added.`); return true; } catch (error) { if (isAlreadyAddedNetworkError(error)) { markNetworkAdded(kind); + if (!silent && !networksCompleteBefore && NETWORKS.every((item) => networkDone(item.kind))) { + trackBuilderStepEvent('journey_step_verified', 'networks'); + } if (!silent) showSuccess(`${network.chainName} already exists in your wallet.`); return true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/BuilderJourney.svelte` around lines 456 - 484, The individual network add flow in addNetwork currently updates local state and success messages, but never emits journey_step_verified when the last missing network is added. Update addNetwork to detect when Bradbury, Asimov, and Studio are all marked added after a successful add or already-added path, and emit journey_step_verified with networks from that function (using markNetworkAdded and the existing tracking logic as the hook). Keep the bulk-add behavior unchanged and ensure the event fires only once when the final required network becomes verified.
626-657: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecompute lifecycle durations after marking role unlock.
claimParamsis built beforemarkLifecycleTime('role_unlocked:builder'), so success/completion/unlock events reuse stale lifecycle timing data.🐛 Proposed fix
await journeyAPI.completeBuilderJourney(); await userStore.loadUser(); markLifecycleTime('role_unlocked:builder'); - trackEvent('builder_role_claim_success', getAnalyticsContext(claimParams)); + const successParams = { + ...claimParams, + ...getLifecycleDurations('builder'), + }; + trackEvent('builder_role_claim_success', getAnalyticsContext(successParams)); trackEvent('journey_completed', getAnalyticsContext({ - ...claimParams, + ...successParams, journey_state: 'completed', })); trackEvent('role_unlocked', getAnalyticsContext({ - ...claimParams, + ...successParams, role_context: 'builder', selected_role: 'builder', surface: 'journey',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/BuilderJourney.svelte` around lines 626 - 657, The lifecycle timing in completeJourney is captured too early, so the success/completion/unlock analytics use stale values. Update BuilderJourney’s completeJourney flow so markLifecycleTime('role_unlocked:builder') happens before collecting lifecycle durations, then rebuild the analytics payload used by trackEvent calls (including builder_role_claim_success, journey_completed, and role_unlocked) with fresh getLifecycleDurations('builder') data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/ProfileCompletionGuard.svelte`:
- Around line 242-246: The post-success reload in ProfileCompletionGuard.svelte
is still inside the failure-sensitive flow, so a loadUser() error can block the
redirect even after updateUserProfile and journey start succeed. Make the
userStore.loadUser() refresh best-effort by catching its failure locally, or
move the push(journeyPath(targetRole)) navigation so it cannot be skipped by a
reload error; keep the redirect logic anchored around updateUserProfile,
userStore.loadUser, and push.
- Around line 232-240: The caught API failure paths in
ProfileCompletionGuard.svelte only track analytics or update state, but they do
not notify the user. Update the relevant try-catch blocks around the journey/API
actions, including the catch for journey start and the other affected API error
handlers in this component, to call showWarning or showError from the toast
store with a user-facing message. Keep the existing analytics/state handling,
and make sure each API failure path surfaces a toast using the existing toast
helpers.
---
Outside diff comments:
In `@frontend/src/components/Sidebar.svelte`:
- Line 269: The Sidebar.svelte link handlers are overriding normal anchor
behavior by calling preventDefault() and navigate(...), which should be removed
for in-app navigation. Update the affected anchor elements in the Sidebar
component to use plain <a href="..."> links and keep any click tracking separate
from routing, so the global interceptor can handle SPA navigation; apply this to
the leaderboard and the other matching links referenced in the Sidebar markup.
In `@frontend/src/routes/BuilderJourney.svelte`:
- Around line 456-484: The individual network add flow in addNetwork currently
updates local state and success messages, but never emits journey_step_verified
when the last missing network is added. Update addNetwork to detect when
Bradbury, Asimov, and Studio are all marked added after a successful add or
already-added path, and emit journey_step_verified with networks from that
function (using markNetworkAdded and the existing tracking logic as the hook).
Keep the bulk-add behavior unchanged and ensure the event fires only once when
the final required network becomes verified.
- Around line 626-657: The lifecycle timing in completeJourney is captured too
early, so the success/completion/unlock analytics use stale values. Update
BuilderJourney’s completeJourney flow so
markLifecycleTime('role_unlocked:builder') happens before collecting lifecycle
durations, then rebuild the analytics payload used by trackEvent calls
(including builder_role_claim_success, journey_completed, and role_unlocked)
with fresh getLifecycleDurations('builder') data.
In `@frontend/src/routes/CommunityJourney.svelte`:
- Around line 425-456: The analytics payload in completeJourney() is built too
early, so the success/completed/unlocked events can omit the newly recorded role
unlock timing. Move the lifecycle timing update and then recompute the claim
analytics context after markLifecycleTime('role_unlocked:community') in
CommunityJourney.svelte, or build a fresh payload for the post-unlock trackEvent
calls so getLifecycleDurations('community') reflects the latest state.
In `@frontend/src/routes/ValidatorWaitlist.svelte`:
- Around line 187-193: The start-failure tracking in ValidatorWaitlist.svelte is
discarding the caught exception, so every journey_start_error is classified as
backend and network failures are misreported. Update the catch block around the
journey start flow to retain the caught error and derive error_stage from it
before calling trackEvent/getAnalyticsContext, using the existing
journey_start_error reporting path and the validator role_context/selected_role
fields so the classification reflects the actual failure type.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f77a4418-a6e0-4d36-a1d1-7dfc1882a36e
📒 Files selected for processing (5)
frontend/src/components/ProfileCompletionGuard.sveltefrontend/src/components/Sidebar.sveltefrontend/src/routes/BuilderJourney.sveltefrontend/src/routes/CommunityJourney.sveltefrontend/src/routes/ValidatorWaitlist.svelte
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/src/components/Sidebar.svelte (1)
269-269: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep these as plain in-app anchors.
These link updates call
preventDefault()and route throughnavigate(...); for normal in-app navigation, leave the<a href="...">behavior intact and track clicks without taking over navigation.As per coding guidelines, “Use plain
<a href="/path">anchors for in-app navigation (global interceptor handles SPA routing) instead of callingpush()in click handlers.”Also applies to: 493-493, 734-734, 923-927
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Sidebar.svelte` at line 269, The Sidebar.svelte link handlers are overriding normal anchor behavior by calling preventDefault() and navigate(...), which should be removed for in-app navigation. Update the affected anchor elements in the Sidebar component to use plain <a href="..."> links and keep any click tracking separate from routing, so the global interceptor can handle SPA navigation; apply this to the leaderboard and the other matching links referenced in the Sidebar markup.Source: Coding guidelines
frontend/src/routes/ValidatorWaitlist.svelte (1)
187-193: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the caught error when classifying start failures.
This always reports
journey_start_error.error_stageasbackend; network failures lose their classification.🐛 Proposed fix
- } catch (_) { + } catch (err) { trackEvent('journey_start_error', getAnalyticsContext({ role_context: 'validator', selected_role: 'validator', surface: 'journey', - error_stage: 'backend', + error_stage: err.response?.status ? 'backend' : 'network', }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/ValidatorWaitlist.svelte` around lines 187 - 193, The start-failure tracking in ValidatorWaitlist.svelte is discarding the caught exception, so every journey_start_error is classified as backend and network failures are misreported. Update the catch block around the journey start flow to retain the caught error and derive error_stage from it before calling trackEvent/getAnalyticsContext, using the existing journey_start_error reporting path and the validator role_context/selected_role fields so the classification reflects the actual failure type.frontend/src/routes/CommunityJourney.svelte (1)
425-456: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecompute lifecycle durations after marking role unlock.
claimParamsis created beforemarkLifecycleTime('role_unlocked:community'), so the success/completed/unlocked analytics can miss the just-recorded unlock timing.🐛 Proposed fix
if (res.data?.user) userStore.updateUser(res.data.user); await userStore.loadUser?.(); markLifecycleTime('role_unlocked:community'); - trackEvent('community_role_claim_success', getAnalyticsContext(claimParams)); + const successParams = { + ...claimParams, + ...getLifecycleDurations('community'), + }; + trackEvent('community_role_claim_success', getAnalyticsContext(successParams)); trackEvent('journey_completed', getAnalyticsContext({ - ...claimParams, + ...successParams, journey_state: 'completed', })); trackEvent('role_unlocked', getAnalyticsContext({ - ...claimParams, + ...successParams, role_context: 'community', selected_role: 'community',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/CommunityJourney.svelte` around lines 425 - 456, The analytics payload in completeJourney() is built too early, so the success/completed/unlocked events can omit the newly recorded role unlock timing. Move the lifecycle timing update and then recompute the claim analytics context after markLifecycleTime('role_unlocked:community') in CommunityJourney.svelte, or build a fresh payload for the post-unlock trackEvent calls so getLifecycleDurations('community') reflects the latest state.frontend/src/routes/BuilderJourney.svelte (2)
456-484: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmit
journey_step_verifiedfor individual network adds.Bulk add reports
networksas verified, but adding Bradbury/Asimov/Studio one-by-one never emits the verified event when the final missing network is added.🐛 Proposed fix
async function addNetwork(network, kind, { silent = false } = {}) { if (typeof window === 'undefined') return; const walletProvider = provider || window.ethereum; if (!walletProvider) { showWarning('Please connect your wallet first.'); return false; } + const networksCompleteBefore = NETWORKS.every((item) => networkDone(item.kind)); if (kind === 'bradbury') isAddingBradbury = true; if (kind === 'asimov') isAddingAsimov = true; if (kind === 'studio') isAddingStudio = true; @@ }); markNetworkAdded(kind); + if (!silent && !networksCompleteBefore && NETWORKS.every((item) => networkDone(item.kind))) { + trackBuilderStepEvent('journey_step_verified', 'networks'); + } if (!silent) showSuccess(`${network.chainName} added.`); return true; } catch (error) { if (isAlreadyAddedNetworkError(error)) { markNetworkAdded(kind); + if (!silent && !networksCompleteBefore && NETWORKS.every((item) => networkDone(item.kind))) { + trackBuilderStepEvent('journey_step_verified', 'networks'); + } if (!silent) showSuccess(`${network.chainName} already exists in your wallet.`); return true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/BuilderJourney.svelte` around lines 456 - 484, The individual network add flow in addNetwork currently updates local state and success messages, but never emits journey_step_verified when the last missing network is added. Update addNetwork to detect when Bradbury, Asimov, and Studio are all marked added after a successful add or already-added path, and emit journey_step_verified with networks from that function (using markNetworkAdded and the existing tracking logic as the hook). Keep the bulk-add behavior unchanged and ensure the event fires only once when the final required network becomes verified.
626-657: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecompute lifecycle durations after marking role unlock.
claimParamsis built beforemarkLifecycleTime('role_unlocked:builder'), so success/completion/unlock events reuse stale lifecycle timing data.🐛 Proposed fix
await journeyAPI.completeBuilderJourney(); await userStore.loadUser(); markLifecycleTime('role_unlocked:builder'); - trackEvent('builder_role_claim_success', getAnalyticsContext(claimParams)); + const successParams = { + ...claimParams, + ...getLifecycleDurations('builder'), + }; + trackEvent('builder_role_claim_success', getAnalyticsContext(successParams)); trackEvent('journey_completed', getAnalyticsContext({ - ...claimParams, + ...successParams, journey_state: 'completed', })); trackEvent('role_unlocked', getAnalyticsContext({ - ...claimParams, + ...successParams, role_context: 'builder', selected_role: 'builder', surface: 'journey',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/BuilderJourney.svelte` around lines 626 - 657, The lifecycle timing in completeJourney is captured too early, so the success/completion/unlock analytics use stale values. Update BuilderJourney’s completeJourney flow so markLifecycleTime('role_unlocked:builder') happens before collecting lifecycle durations, then rebuild the analytics payload used by trackEvent calls (including builder_role_claim_success, journey_completed, and role_unlocked) with fresh getLifecycleDurations('builder') data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/ProfileCompletionGuard.svelte`:
- Around line 242-246: The post-success reload in ProfileCompletionGuard.svelte
is still inside the failure-sensitive flow, so a loadUser() error can block the
redirect even after updateUserProfile and journey start succeed. Make the
userStore.loadUser() refresh best-effort by catching its failure locally, or
move the push(journeyPath(targetRole)) navigation so it cannot be skipped by a
reload error; keep the redirect logic anchored around updateUserProfile,
userStore.loadUser, and push.
- Around line 232-240: The caught API failure paths in
ProfileCompletionGuard.svelte only track analytics or update state, but they do
not notify the user. Update the relevant try-catch blocks around the journey/API
actions, including the catch for journey start and the other affected API error
handlers in this component, to call showWarning or showError from the toast
store with a user-facing message. Keep the existing analytics/state handling,
and make sure each API failure path surfaces a toast using the existing toast
helpers.
---
Outside diff comments:
In `@frontend/src/components/Sidebar.svelte`:
- Line 269: The Sidebar.svelte link handlers are overriding normal anchor
behavior by calling preventDefault() and navigate(...), which should be removed
for in-app navigation. Update the affected anchor elements in the Sidebar
component to use plain <a href="..."> links and keep any click tracking separate
from routing, so the global interceptor can handle SPA navigation; apply this to
the leaderboard and the other matching links referenced in the Sidebar markup.
In `@frontend/src/routes/BuilderJourney.svelte`:
- Around line 456-484: The individual network add flow in addNetwork currently
updates local state and success messages, but never emits journey_step_verified
when the last missing network is added. Update addNetwork to detect when
Bradbury, Asimov, and Studio are all marked added after a successful add or
already-added path, and emit journey_step_verified with networks from that
function (using markNetworkAdded and the existing tracking logic as the hook).
Keep the bulk-add behavior unchanged and ensure the event fires only once when
the final required network becomes verified.
- Around line 626-657: The lifecycle timing in completeJourney is captured too
early, so the success/completion/unlock analytics use stale values. Update
BuilderJourney’s completeJourney flow so
markLifecycleTime('role_unlocked:builder') happens before collecting lifecycle
durations, then rebuild the analytics payload used by trackEvent calls
(including builder_role_claim_success, journey_completed, and role_unlocked)
with fresh getLifecycleDurations('builder') data.
In `@frontend/src/routes/CommunityJourney.svelte`:
- Around line 425-456: The analytics payload in completeJourney() is built too
early, so the success/completed/unlocked events can omit the newly recorded role
unlock timing. Move the lifecycle timing update and then recompute the claim
analytics context after markLifecycleTime('role_unlocked:community') in
CommunityJourney.svelte, or build a fresh payload for the post-unlock trackEvent
calls so getLifecycleDurations('community') reflects the latest state.
In `@frontend/src/routes/ValidatorWaitlist.svelte`:
- Around line 187-193: The start-failure tracking in ValidatorWaitlist.svelte is
discarding the caught exception, so every journey_start_error is classified as
backend and network failures are misreported. Update the catch block around the
journey start flow to retain the caught error and derive error_stage from it
before calling trackEvent/getAnalyticsContext, using the existing
journey_start_error reporting path and the validator role_context/selected_role
fields so the classification reflects the actual failure type.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f77a4418-a6e0-4d36-a1d1-7dfc1882a36e
📒 Files selected for processing (5)
frontend/src/components/ProfileCompletionGuard.sveltefrontend/src/components/Sidebar.sveltefrontend/src/routes/BuilderJourney.sveltefrontend/src/routes/CommunityJourney.sveltefrontend/src/routes/ValidatorWaitlist.svelte
🛑 Comments failed to post (2)
frontend/src/components/ProfileCompletionGuard.svelte (2)
232-240: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show toast feedback for API failures.
The new/updated API failure paths only emit analytics or set inline state. Add
showWarning/showErrorfrom the toast store for these caught API errors.As per coding guidelines, “Always handle API errors with try-catch blocks and provide user-facing error messages via toast notifications.”
Also applies to: 247-265
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ProfileCompletionGuard.svelte` around lines 232 - 240, The caught API failure paths in ProfileCompletionGuard.svelte only track analytics or update state, but they do not notify the user. Update the relevant try-catch blocks around the journey/API actions, including the catch for journey start and the other affected API error handlers in this component, to call showWarning or showError from the toast store with a user-facing message. Keep the existing analytics/state handling, and make sure each API failure path surfaces a toast using the existing toast helpers.Source: Coding guidelines
242-246: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don’t let the post-success reload strand the user.
After
updateUserProfileand journey start succeed,await userStore.loadUser()can still fail and fall into the outer “profile completion failed” path, preventing the redirect even though the profile was already saved. Make the reload best-effort or move navigation out of this failure path.Also applies to: 247-251
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ProfileCompletionGuard.svelte` around lines 242 - 246, The post-success reload in ProfileCompletionGuard.svelte is still inside the failure-sensitive flow, so a loadUser() error can block the redirect even after updateUserProfile and journey start succeed. Make the userStore.loadUser() refresh best-effort by catching its failure locally, or move the push(journeyPath(targetRole)) navigation so it cannot be skipped by a reload error; keep the redirect logic anchored around updateUserProfile, userStore.loadUser, and push.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds privacy-safe Google Analytics tracking across onboarding, role journeys, validator waitlist, contribution submission, and submission lifecycle surfaces.
Centralizes sanitization, route templating, lifecycle timers, and one-shot events so funnel metrics can be measured without sending raw user identifiers or free-text content.
Tightens event attribution for dynamic page views, OAuth journey links, click-through tasks, and deep-linked contribution forms.
Summary by CodeRabbit
New Features
Bug Fixes
Tests