Skip to content

feat: add canonical onboarding funnel events for Mixpanel#2000

Open
remicolin wants to merge 4 commits intodevfrom
analytics/canonical-onboarding-funnel
Open

feat: add canonical onboarding funnel events for Mixpanel#2000
remicolin wants to merge 4 commits intodevfrom
analytics/canonical-onboarding-funnel

Conversation

@remicolin
Copy link
Copy Markdown
Collaborator

@remicolin remicolin commented Apr 20, 2026

Introduces a thin canonical event layer on top of the existing diagnostic events so the Mixpanel onboarding funnel can measure drop-off with statistical confidence. The ~200 pre-existing events remain untouched.

Canonical events fire at most once per attempt, guarded by a shared helper in mobile-sdk-alpha rather than component lifecycle — back-nav no longer re-fires step events. Every canonical event carries a branch property (biometric_passport | biometric_id | kyc | aadhaar) so a single macro funnel and per-branch diagnostic funnels can both be built from one stream.

Terminal invariant: onboarding_completed fires only when the proving machine reaches completed via a new registration proof this session (tracked via didNewRegistrationProof). The ALREADY_REGISTERED shortcut and any disclose flow do not trigger it — protects the funnel's terminal event from disclosure-later pollution.

Also fixes the AbstractButton Click: prefix for canonical events via a new trackEventRaw prop (existing trackEvent consumers unchanged), and instruments the four dead-zone screens (LogoConfirmation, CountryPicker, IDSelection, registration fallbacks).

Spec: specs/projects/sdk/workstreams/analytics/SPEC.md
Plan: specs/projects/sdk/workstreams/analytics/plans/ANA-01-canonical-onboarding-funnel.md

Deferred to follow-up issues (ANA-02/03/04):

  • TestFlight / internal-build contamination filtering
  • Raw-string event cleanup (REGISTRATION_FALLBACK_, DEVICE_TOKEN_REG_)
  • Mixpanel NFC native channel vs Segment consolidation

Summary

Test plan


Native Consolidation Checklist

  • CONTRACTS.md reviewed - no unintended contract changes
  • Layer 1 bridge contract tests pass (cd app && yarn jest:run / yarn workspace @selfxyz/rn-sdk-test-app test)
  • Layer 3 builds pass (app iOS, RN test app iOS, RN test app Android)
  • Layer 4 manual smoke test signed off (if consolidation PR)
  • No new native business logic added (logic belongs in TypeScript)

Summary by CodeRabbit

  • Chores

    • Enhanced onboarding analytics infrastructure with canonical funnel event tracking across document verification flows, including scan initiation/completion and proof generation stages.
    • Improved analytics event handling for verification workflows.
  • Tests

    • Added comprehensive test suite for onboarding funnel analytics behavior.
  • Documentation

    • Added analytics implementation specifications and workstream documentation.

Introduces a thin canonical event layer on top of the existing diagnostic
events so the Mixpanel onboarding funnel can measure drop-off with
statistical confidence. The ~200 pre-existing events remain untouched.

Canonical events fire at most once per attempt, guarded by a shared
helper in mobile-sdk-alpha rather than component lifecycle — back-nav no
longer re-fires step events. Every canonical event carries a `branch`
property (biometric_passport | biometric_id | kyc | aadhaar) so a single
macro funnel and per-branch diagnostic funnels can both be built from
one stream.

Terminal invariant: `onboarding_completed` fires only when the proving
machine reaches `completed` via a new registration proof this session
(tracked via `didNewRegistrationProof`). The `ALREADY_REGISTERED`
shortcut and any `disclose` flow do not trigger it — protects the
funnel's terminal event from disclosure-later pollution.

Also fixes the AbstractButton `Click:` prefix for canonical events via a
new `trackEventRaw` prop (existing `trackEvent` consumers unchanged),
and instruments the four dead-zone screens (LogoConfirmation,
CountryPicker, IDSelection, registration fallbacks).

Spec: specs/projects/sdk/workstreams/analytics/SPEC.md
Plan: specs/projects/sdk/workstreams/analytics/plans/ANA-01-canonical-onboarding-funnel.md

Deferred to follow-up issues (ANA-02/03/04):
- TestFlight / internal-build contamination filtering
- Raw-string event cleanup (REGISTRATION_FALLBACK_*, DEVICE_TOKEN_REG_*)
- Mixpanel NFC native channel vs Segment consolidation

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
self-webview-app Ready Ready Preview, Comment Apr 23, 2026 0:32am

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f5b401ff-87c0-4226-98c1-8dc0a06fb63d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR introduces a comprehensive onboarding funnel analytics system across the mobile SDK and application. It adds a new state-machine-based onboarding attempt tracker that records canonical funnel events (started, country/document selection, scan progression, proof generation, completion), integrates this tracking into existing screens and flows, and extends the SDK's public API with onboarding types and control functions.

Changes

Cohort / File(s) Summary
App Screen Instrumentation
app/src/screens/documents/aadhaar/AadhaarUploadScreen.tsx, app/src/screens/documents/management/ManageDocumentsScreen.tsx, app/src/screens/documents/scanning/DocumentCameraScreen.tsx, app/src/screens/documents/scanning/DocumentNFCScanScreen.tsx, app/src/screens/documents/scanning/RegistrationFallback*.tsx, app/src/screens/documents/selection/LogoConfirmationScreen.tsx, app/src/screens/onboarding/DisclaimerScreen.tsx
Multiple screens updated to obtain selfClient and emit onboarding analytics events (e.g., SCAN_STARTED, SCAN_SUCCEEDED, COUNTRY_SELECTED, branch/funnel tracking) at appropriate flow entry/completion points; dependency arrays extended to include selfClient.
SDK Onboarding Funnel Core
packages/mobile-sdk-alpha/src/analytics/onboardingFunnel.ts
New module implementing internal onboarding attempt state machine with deduplication, retry counting, and module-level attempt tracking; exports types (OnboardingBranch, OnboardingStage, OnboardingFailureStage) and functions for attempt lifecycle (startOnboardingAttempt, trackOnboardingStep, trackOnboardingRetry, completeOnboardingAttempt, failOnboardingAttempt) plus utilities (resolveOnboardingBranch, setOnboardingBranch).
SDK Analytics Constants
packages/mobile-sdk-alpha/src/constants/analytics.ts
Added new OnboardingEvents constant object defining canonical Mixpanel funnel event names (e.g., STARTED, COUNTRY_SELECTED, SCAN_STARTED, COMPLETED, FAILED, STEP_RETRIED).
SDK Component Extensions
packages/mobile-sdk-alpha/src/components/buttons/AbstractButton.tsx
Extended ButtonProps with trackEventRaw and trackEventProperties to allow buttons to emit unmodified event names; added conditional logic to bypass legacy event name prefixing when trackEventRaw is provided.
SDK Flow Screens
packages/mobile-sdk-alpha/src/flows/onboarding/country-picker-screen.tsx, packages/mobile-sdk-alpha/src/flows/onboarding/id-selection-screen.tsx
Built-in SDK flow screens updated to call trackOnboardingStep when country/document type selections occur, including branch resolution and document type normalization.
SDK State Machine
packages/mobile-sdk-alpha/src/proving/provingMachine.ts
Extended ProvingState with didNewRegistrationProof flag; added onboarding step tracking on proof entry, funnel success/failure event emission on state transitions, with branch-specific logic for registration vs. disclosure flows.
SDK Public Exports
packages/mobile-sdk-alpha/src/index.ts
Added re-exports of onboarding types (OnboardingBranch, OnboardingStage, OnboardingFailureStage) and functions (attempt lifecycle, retry/step tracking, test helpers) from onboardingFunnel module.
Tests
packages/mobile-sdk-alpha/tests/analytics/onboardingFunnel.test.ts
New Vitest suite verifying branch resolution, attempt creation/completion, step deduplication, retry counting, and funnel state transitions with mocked trackEvent client.
Documentation
specs/projects/sdk/INDEX.md, specs/projects/sdk/workstreams/analytics/SPEC.md, specs/projects/sdk/workstreams/analytics/plans/ANA-01-canonical-onboarding-funnel.md
Added analytics workstream overview, detailed funnel specification (event layers, properties, branch model, invariants), and implementation plan with execution steps and done criteria.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description addresses the template sections but lacks explicit test plan documentation; however, the description is comprehensive and detailed about the changes, objectives, and technical approach. Add a 'Test plan' section describing how the changes were validated (e.g., unit tests, integration tests, manual verification steps).
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: adding canonical onboarding funnel events for Mixpanel analytics.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch analytics/canonical-onboarding-funnel

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)

532-555: ⚠️ Potential issue | 🟠 Major

Terminalize DSC registration proof failures in the onboarding funnel.

The DSC leg is handled as a registration-flow error elsewhere (circuitType !== 'disclose'), but these failure branches only call failOnboardingAttempt() for register. If the DSC proof fails before switching to register, the onboarding attempt remains active with no failed terminal event.

Proposed fix
-        } else if (get().circuitType === 'register') {
+        } else if (get().circuitType === 'register' || get().circuitType === 'dsc') {
           failOnboardingAttempt(selfClient, 'proof_generation_started', reason ?? error_code ?? 'proof_failure', {
             recoverable: false,
           });
         }
@@
-        } else if (get().circuitType === 'register') {
+        } else if (get().circuitType === 'register' || get().circuitType === 'dsc') {
           failOnboardingAttempt(selfClient, 'proof_generation_started', get().reason ?? get().error_code ?? 'error', {
             recoverable: true,
           });
         }

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 80621fbc-d783-41b9-b5b2-52901e031aeb

📥 Commits

Reviewing files that changed from the base of the PR and between 132ba87 and 6414da4.

📒 Files selected for processing (19)
  • app/src/screens/documents/aadhaar/AadhaarUploadScreen.tsx
  • app/src/screens/documents/management/ManageDocumentsScreen.tsx
  • app/src/screens/documents/scanning/DocumentCameraScreen.tsx
  • app/src/screens/documents/scanning/DocumentNFCScanScreen.tsx
  • app/src/screens/documents/scanning/RegistrationFallbackMRZScreen.tsx
  • app/src/screens/documents/scanning/RegistrationFallbackNFCScreen.tsx
  • app/src/screens/documents/selection/LogoConfirmationScreen.tsx
  • app/src/screens/onboarding/DisclaimerScreen.tsx
  • packages/mobile-sdk-alpha/src/analytics/onboardingFunnel.ts
  • packages/mobile-sdk-alpha/src/components/buttons/AbstractButton.tsx
  • packages/mobile-sdk-alpha/src/constants/analytics.ts
  • packages/mobile-sdk-alpha/src/flows/onboarding/country-picker-screen.tsx
  • packages/mobile-sdk-alpha/src/flows/onboarding/id-selection-screen.tsx
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/proving/provingMachine.ts
  • packages/mobile-sdk-alpha/tests/analytics/onboardingFunnel.test.ts
  • specs/projects/sdk/INDEX.md
  • specs/projects/sdk/workstreams/analytics/SPEC.md
  • specs/projects/sdk/workstreams/analytics/plans/ANA-01-canonical-onboarding-funnel.md

Comment on lines +388 to +391
trackOnboardingStep(selfClient, OnboardingEvents.SCAN_SUCCEEDED, {
branch: resolveOnboardingBranch(documentType ?? 'p'),
duration_seconds: parseFloat(scanDurationSeconds),
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Move the canonical scan success event after the scan is committed.

document_scan_succeeded fires before parseScanResponse(scanResponse) and storePassportData(passportData). If parsing or storage fails, this attempt is still counted as a successful scan and the fire-once guard can block the real success on retry. Emit this only after the passport data has been parsed and stored successfully.

🐛 Proposed fix
-        trackOnboardingStep(selfClient, OnboardingEvents.SCAN_SUCCEEDED, {
-          branch: resolveOnboardingBranch(documentType ?? 'p'),
-          duration_seconds: parseFloat(scanDurationSeconds),
-        });
         logNFCEvent(
           'info',
           'scan_success',
@@
         if (passportData) {
           console.log('Storing passport data from NFC scan...');
           await storePassportData(passportData);
           console.log('Passport data stored successfully');
+          trackOnboardingStep(selfClient, OnboardingEvents.SCAN_SUCCEEDED, {
+            branch: resolveOnboardingBranch(documentType ?? 'p'),
+            duration_seconds: parseFloat(scanDurationSeconds),
+          });
         }
📝 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.

Suggested change
trackOnboardingStep(selfClient, OnboardingEvents.SCAN_SUCCEEDED, {
branch: resolveOnboardingBranch(documentType ?? 'p'),
duration_seconds: parseFloat(scanDurationSeconds),
});
logNFCEvent(
'info',
'scan_success',
);
const passportData = parseScanResponse(scanResponse);
if (passportData) {
console.log('Storing passport data from NFC scan...');
await storePassportData(passportData);
console.log('Passport data stored successfully');
trackOnboardingStep(selfClient, OnboardingEvents.SCAN_SUCCEEDED, {
branch: resolveOnboardingBranch(documentType ?? 'p'),
duration_seconds: parseFloat(scanDurationSeconds),
});
}

Comment on lines +46 to +57
function ensureAttempt(): OnboardingAttempt {
if (!currentAttempt) {
currentAttempt = {
id: uuid(),
branch: 'pending',
startedAt: Date.now(),
firedSteps: new Set(),
retryCounts: {},
};
}
return currentAttempt;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Emit STARTED when bootstrapping a deep-link attempt.

ensureAttempt() creates an attempt silently, so a deep-link entry can emit COUNTRY_SELECTED, STEP_RETRIED, or later proof events with an attempt_id but no matching Onboarding: Started. That undercounts top-of-funnel starts and leaves the attempt lifecycle inconsistent.

Proposed fix
-function ensureAttempt(): OnboardingAttempt {
+function ensureAttempt(selfClient: Pick<SelfClient, 'trackEvent'>): OnboardingAttempt {
   if (!currentAttempt) {
     currentAttempt = {
       id: uuid(),
       branch: 'pending',
       startedAt: Date.now(),
       firedSteps: new Set(),
       retryCounts: {},
     };
+    currentAttempt.firedSteps.add(OnboardingEvents.STARTED);
+    selfClient.trackEvent(OnboardingEvents.STARTED, baseProperties(currentAttempt));
   }
   return currentAttempt;
 }
@@
-  const attempt = ensureAttempt();
+  const attempt = ensureAttempt(selfClient);
@@
-  const attempt = ensureAttempt();
+  const attempt = ensureAttempt(selfClient);

Also applies to: 205-219, 230-254

Comment on lines +459 to +464
if (state.value === 'proving') {
// Canonical funnel: fire-once per attempt. `trackOnboardingStep`
// dedupes, so transient re-entry into `proving` (e.g. via reconnect)
// does not re-emit.
trackOnboardingStep(selfClient, OnboardingEvents.PROOF_STARTED);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Gate canonical onboarding proof-start events away from disclosure flows.

Line 463 runs for disclose too. Since trackOnboardingStep() bootstraps an onboarding attempt and the disclosure completion path only sends DISCLOSURE_COMPLETED, disclosure proofs can leave a stale onboarding attempt open and pollute the next funnel.

Proposed fix
-      if (state.value === 'proving') {
+      if (state.value === 'proving' && get().circuitType !== 'disclose') {
         // Canonical funnel: fire-once per attempt. `trackOnboardingStep`
         // dedupes, so transient re-entry into `proving` (e.g. via reconnect)
         // does not re-emit.
         trackOnboardingStep(selfClient, OnboardingEvents.PROOF_STARTED);
       }

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 20, 2026

Greptile Summary

This PR introduces a thin canonical analytics layer (onboardingFunnel.ts) on top of the existing ~200 diagnostic events, enabling a Mixpanel onboarding funnel with statistically reliable drop-off measurement. Key additions include a fire-once-per-attempt deduplication guard, a branch dimension (biometric_passport | biometric_id | kyc | aadhaar) on every event, and a didNewRegistrationProof flag in the proving machine that prevents the terminal onboarding_completed event from firing on ALREADY_REGISTERED shortcuts or disclose flows. The AbstractButton trackEventRaw prop unblocks canonical events from the existing Click: prefix mangling without touching the ~200 existing consumers.

Issues found:

  • Double Onboarding: Started for first-time usersDisclaimerScreen calls startOnboardingAttempt and then ManageDocumentsScreen.handleAddDocument calls it again seconds later when the user taps "Add Document." First-time users generate two Onboarding: Started events with different attempt_ids; the first is silently abandoned, inflating funnel entry counts and producing orphaned events with no subsequent steps.

  • SCAN_SUCCEEDED funnel gap for the biometric → KYC fallback pathRegistrationFallbackMRZScreen.handleTryAlternative and RegistrationFallbackNFCScreen.handleTryAlternative both call setOnboardingBranch('kyc') and launch KYC verification, but never fire SCAN_SUCCEEDED on success. Because SCAN_STARTED was already deduped from the biometric camera mount, these users fall out of the Mixpanel funnel between SCAN_STARTED and PROOF_STARTED despite completing registration.

  • Proving-machine terminal invariant tests are missing — the spec explicitly requires integration tests verifying that onboarding_completed fires on a new registration, does not fire on ALREADY_REGISTERED, and does not fire on disclose. The didNewRegistrationProof gate has no automated regression coverage.

Confidence Score: 3/5

Not safe to merge without addressing the double-STARTED event and the SCAN_SUCCEEDED gap in fallback screens — both produce silent funnel data corruption from day one.

The core mechanics are solid — the dedup guard, didNewRegistrationProof gate, and AbstractButton fix are all well-designed. However, two P1 logic bugs actively corrupt the funnel data this PR is specifically designed to produce: first-time users generate double STARTED events (inflating entry counts), and biometric→KYC fallback users silently drop out of the funnel between SCAN_STARTED and PROOF_STARTED. Since the PR's sole purpose is accurate funnel measurement, shipping these gaps defeats the goal before the dashboard is even built.

app/src/screens/onboarding/DisclaimerScreen.tsx (double STARTED), app/src/screens/documents/scanning/RegistrationFallbackMRZScreen.tsx and RegistrationFallbackNFCScreen.tsx (missing SCAN_SUCCEEDED for KYC fallback path).

Important Files Changed

Filename Overview
packages/mobile-sdk-alpha/src/analytics/onboardingFunnel.ts New canonical funnel module — well-structured with dedup guard, branch tracking, and terminal completion/failure helpers; module-level singleton is appropriate for mobile.
packages/mobile-sdk-alpha/src/proving/provingMachine.ts Adds didNewRegistrationProof flag to gate canonical terminal events; correctly prevents ALREADY_REGISTERED shortcut and disclose flows from firing onboarding_completed.
app/src/screens/onboarding/DisclaimerScreen.tsx Adds startOnboardingAttempt on disclaimer dismiss — but ManageDocumentsScreen.handleAddDocument also calls it, causing first-time users to emit two Onboarding: Started events with different attempt_ids, inflating funnel entry counts.
app/src/screens/documents/scanning/RegistrationFallbackMRZScreen.tsx Sets branch to KYC on fallback but never fires SCAN_SUCCEEDED when KYC succeeds, creating a funnel dead-zone for biometric→KYC fallback users.
app/src/screens/documents/scanning/RegistrationFallbackNFCScreen.tsx Same SCAN_SUCCEEDED gap as RegistrationFallbackMRZScreen — KYC fallback path from NFC failure doesn't track scan success, leaving funnel incomplete for this path.
packages/mobile-sdk-alpha/src/components/buttons/AbstractButton.tsx Adds trackEventRaw prop as an additive opt-in to bypass the Click: prefix mangling; backward compatible with existing trackEvent consumers.
app/src/screens/documents/selection/LogoConfirmationScreen.tsx Correctly adds button tracking for logo confirmation answers and explicitly fires both SCAN_STARTED and SCAN_SUCCEEDED for the KYC fallback path — consistent pattern.
packages/mobile-sdk-alpha/tests/analytics/onboardingFunnel.test.ts Thorough unit test coverage for all public API in onboardingFunnel.ts; missing the proving-machine terminal invariant integration tests required by the spec.

Sequence Diagram

sequenceDiagram
    participant D as DisclaimerScreen
    participant M as ManageDocumentsScreen
    participant CP as CountryPickerScreen
    participant ID as IDSelectionScreen
    participant LC as LogoConfirmationScreen
    participant DC as DocumentCameraScreen
    participant NFC as DocumentNFCScanScreen
    participant RF as RegistrationFallbackScreen
    participant PM as ProvingMachine
    participant OF as onboardingFunnel.ts

    D->>OF: startOnboardingAttempt() → STARTED (attempt A)
    D->>M: navigate Home
    M->>OF: startOnboardingAttempt() → STARTED (attempt B, abandons A)
    M->>CP: navigate CountryPicker
    CP->>OF: trackOnboardingStep(COUNTRY_SELECTED)
    CP->>ID: navigate IDSelection
    ID->>OF: trackOnboardingStep(DOCUMENT_TYPE_SELECTED, branch=biometric_passport)
    ID->>LC: navigate LogoConfirmation
    alt Logo Confirmed (Yes)
        LC->>DC: navigate DocumentCamera
        DC->>OF: trackOnboardingStep(SCAN_STARTED, branch=biometric_passport)
        DC->>NFC: navigate DocumentNFCScan
        NFC->>OF: trackOnboardingStep(SCAN_SUCCEEDED, branch=biometric_*)
        NFC->>PM: proof flow begins
        PM->>OF: trackOnboardingStep(PROOF_STARTED)
        PM->>OF: trackOnboardingStep(PROOF_SUCCEEDED)
        PM->>OF: completeOnboardingAttempt() → COMPLETED
    else Logo Not Found (No → KYC)
        LC->>OF: setOnboardingBranch('kyc')
        LC->>OF: trackOnboardingStep(SCAN_STARTED, branch=kyc)
        LC->>OF: trackOnboardingStep(SCAN_SUCCEEDED, branch=kyc)
        LC->>PM: proof flow begins
    else MRZ/NFC Scan Failed → Fallback → Try Alternative (KYC)
        RF->>OF: setOnboardingBranch('kyc')
        Note over RF,OF: ⚠️ SCAN_SUCCEEDED never fires here!
        RF->>PM: launchKycVerification()
    end
Loading

Comments Outside Diff (1)

  1. app/src/screens/documents/scanning/RegistrationFallbackMRZScreen.tsx, line 90-96 (link)

    P1 Missing SCAN_SUCCEEDED canonical event in the biometric → KYC fallback path

    When a user's MRZ scan fails and they choose "Try Alternative" (KYC), setOnboardingBranch('kyc') is called to update the branch, but no SCAN_SUCCEEDED canonical event is ever fired if the KYC verification succeeds.

    Because SCAN_STARTED was already deduped from DocumentCameraScreen's mount effect, it cannot fire again for the KYC launch. The result is that users who reach registration via this fallback path produce a funnel that stops at SCAN_STARTED (biometric), skipping SCAN_SUCCEEDED, and then jumps directly to PROOF_STARTED — dropping them as funnel non-converters between those two steps despite successfully completing.

    The same gap exists in RegistrationFallbackNFCScreen.handleTryAlternative (lines 96–103 of that file).

    By contrast, LogoConfirmationScreen.handleNotFound explicitly fires both SCAN_STARTED and SCAN_SUCCEEDED for the KYC path, making the fallback screens inconsistent.

    Consider tracking SCAN_SUCCEEDED (with branch: 'kyc') in the success result handler after launchKycVerification() returns successfully in both fallback screens — matching the pattern used in LogoConfirmationScreen.

Reviews (1): Last reviewed commit: "feat: add canonical onboarding funnel ev..." | Re-trigger Greptile

Comment on lines 73 to 74
startOnboardingAttempt(selfClient);
navigation.navigate({ name: 'Home', params: {} });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Double Onboarding: Started event for first-time users

For first-time users, startOnboardingAttempt is called here in DisclaimerScreen (emitting Onboarding: Started with attempt_id: A), and then called again in ManageDocumentsScreen.handleAddDocument (emitting another Onboarding: Started with attempt_id: B, silently abandoning A). The user ends up in Mixpanel as having entered the funnel twice with two unrelated attempt_ids — only the second one carries the downstream step events (country_selected, document_type_selected, etc.).

Returning users (who don't see the disclaimer) get one STARTED event; first-time users get two. This inflates the funnel's entry count for first-time cohorts and produces orphaned onboarding_started events with no follow-up steps.

Root cause: The spec places the onboarding_started entrypoint on the disclaimer dismiss, but ManageDocumentsScreen is also an entry point (for returning users). Both paths call startOnboardingAttempt, so first-time users — who go through both screens in sequence — hit both.

Suggested fix: Remove the startOnboardingAttempt call from DisclaimerScreen and rely solely on ManageDocumentsScreen.handleAddDocument as the canonical funnel entry. The disclaimer dismiss is a one-time gate; the funnel should start when the user commits to scanning a document.

Suggested change
startOnboardingAttempt(selfClient);
navigation.navigate({ name: 'Home', params: {} });
dismissPrivacyNote();
navigation.navigate({ name: 'Home', params: {} });

Comment on lines +1 to +5
// SPDX-FileCopyrightText: 2025-2026 Social Connect Labs, Inc.
// SPDX-License-Identifier: BUSL-1.1
// NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE.

import { beforeEach, describe, expect, it, vi } from 'vitest';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing terminal invariant tests for the proving machine

The spec (SPEC.md / ANA-01-canonical-onboarding-funnel.md) explicitly requires:

Integration test for the proving-machine terminal invariant (fires on new registration, does not fire on ALREADY_REGISTERED, does not fire on disclose).

The existing test file covers onboardingFunnel.ts unit behaviour thoroughly, but there are no tests verifying the didNewRegistrationProof gate in provingMachine.ts. The spec mentions these should live at packages/mobile-sdk-alpha/src/proving/__tests__/provingMachine.analytics.test.ts.

The invariant is the most critical correctness guarantee of the PR — if completeOnboardingAttempt fires on the ALREADY_REGISTERED shortcut or on a disclose flow, the onboarding_completed event would be permanently polluted with non-registration completions. Without automated regression coverage this invariant is easy to accidentally break in a future proving-machine refactor.

…backs

Splits the single `branch` property into `initial_branch` (user's original
intent, immutable for the attempt) and `current_branch` (updated when the
user falls back from biometric to KYC mid-flow via setOnboardingBranch).
Terminal events additionally stamp `used_fallback: boolean`.

Without this split, a user who starts biometric and falls back to KYC gets
later events stamped with branch=kyc, breaking both the biometric funnel
(appears as a drop-off at scan) and the KYC funnel (appears as an entry
without document_type_selected). With the split:

  initial_branch = biometric_passport   →  "what did the user intend"
  current_branch = kyc                  →  "what did they actually complete"
  used_fallback  = true                 →  easy cohort for fallback analysis

Also documents the three-layer event model (canonical funnel, canonical
decision events, diagnostic) and adds six new backlog issues covering the
gap between "measurable funnel" and "Revolut-grade funnel":

  ANA-05  fallback decision events (Layer 2) — next priority after ANA-01
  ANA-06  super-property enrichment (device, OS, version, channel)
  ANA-07  step-view vs step-commit mini-funnels
  ANA-08  abandonment events on app-background
  ANA-09  A/B test tagging at the super-property layer
  ANA-10  PM dashboard roll-ups (D1/D7/D30 conversion, TTV, top drops)

Spec: SPEC.md § Cross-branch flows, § What This Doesn't Measure Yet

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…boarding-funnel

# Conflicts:
#	app/src/screens/documents/scanning/DocumentCameraScreen.tsx
#	packages/mobile-sdk-alpha/src/proving/provingMachine.ts
…er-entry

Moves the STARTED emission into the funnel helper's `ensureAttempt`
bootstrap so it fires exactly once per attempt at the first canonical
step event (typically `country_selected`), regardless of which entry
path the user took. Deletes the two explicit `startOnboardingAttempt`
call sites.

Closes the P1 Greptile finding on PR #2000. Before this change:

  First-time via Home EmptyIdCard           1 STARTED  ✓
  First-time via ManageDocuments            2 STARTED  ✗ (Greptile bug)
  Returning via HomeNavBar +                0 STARTED  ✗ (silent bootstrap)
  Returning via ManageDocuments             1 STARTED  ✓
  Recovery / RecoverWithPhrase              0 STARTED  ✗
  CloudBackupScreen "Register now"          0 STARTED  ✗

After:

  Every entry path                          1 STARTED  ✓  (by construction)

Removes the public `startOnboardingAttempt` API entirely — no callers
needed it once the bootstrap carries the emission. The dual-creation-
paths footgun is gone.

Trade-off accepted: `duration_seconds` on `onboarding_completed` now
measures from the first canonical step (typically country_selected)
rather than from privacy-disclaimer dismiss. The disclaimer is a
one-time legal gate unrelated to registration effort, and a
"dismissed-but-didn't-start" signal belongs in an app-engagement
funnel, not the registration funnel.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant