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 onboarding funnel layer on top of the existing ~200 diagnostic events, enabling Mixpanel to measure drop-off with statistical confidence. The new onboardingFunnel.ts module uses lazy-bootstrap + per-attempt deduplication so every canonical event fires at most once per attempt regardless of back-navigation or re-mounts. The didNewRegistrationProof flag in provingMachine.ts correctly guards the terminal onboarding_completed event against the ALREADY_REGISTERED shortcut and all disclose flows.

Key changes:

  • New onboardingFunnel.ts module with trackOnboardingStep, trackOnboardingRetry, completeOnboardingAttempt, failOnboardingAttempt, setOnboardingBranch, and resolveOnboardingBranch — all exported and covered by a thorough unit-test suite.
  • OnboardingEvents constants group added alongside the existing diagnostic groups.
  • provingMachine.ts wired for PROOF_STARTED, PROOF_SUCCEEDED, COMPLETED, and FAILED with the didNewRegistrationProof terminal invariant.
  • Four previously un-instrumented screens now emit canonical step events.
  • Issue found: RegistrationFallbackMRZScreen.handleTryAlternative and RegistrationFallbackNFCScreen.handleTryAlternative both update the branch to kyc and await launchKycVerification() but never fire SCAN_SUCCEEDED on a successful KYC session. Users who fail biometric scanning and successfully recover via KYC from either fallback screen will appear as dropped off at scan_started in the funnel.

Confidence Score: 4/5

Safe to merge; core funnel invariants and deduplication are correct. One concrete data-quality fix remains for the biometric-failure → KYC recovery cohort.

The architecture is sound — the lazy-bootstrap pattern resolves the double-STARTED issue from the previous review thread, the terminal invariant is correctly guarded by didNewRegistrationProof, and the unit test suite is comprehensive. The single remaining issue (missing SCAN_SUCCEEDED in both RegistrationFallback screens' KYC paths) is a real data gap that will produce misleading drop-off metrics for that cohort, but does not break user flows or cause data loss.

app/src/screens/documents/scanning/RegistrationFallbackMRZScreen.tsx and RegistrationFallbackNFCScreen.tsx — handleTryAlternative needs SCAN_SUCCEEDED tracking after successful KYC.

Important Files Changed

Filename Overview
packages/mobile-sdk-alpha/src/analytics/onboardingFunnel.ts New canonical funnel module; lazy bootstrap via ensureAttempt, per-attempt deduplication via firedSteps, correct branch tracking for cross-branch fallbacks.
packages/mobile-sdk-alpha/src/proving/provingMachine.ts Adds didNewRegistrationProof flag to gate completeOnboardingAttempt; ALREADY_REGISTERED shortcut correctly prevented from firing terminal event.
app/src/screens/documents/scanning/RegistrationFallbackMRZScreen.tsx handleTryAlternative sets branch to KYC and calls launchKycVerification but never fires SCAN_SUCCEEDED on success, creating a funnel gap.
app/src/screens/documents/scanning/RegistrationFallbackNFCScreen.tsx Same SCAN_SUCCEEDED omission as RegistrationFallbackMRZScreen for the handleTryAlternative → KYC path.
packages/mobile-sdk-alpha/tests/analytics/onboardingFunnel.test.ts Comprehensive unit coverage of onboardingFunnel.ts: bootstrap, deduplication, branch locking, fallback, retry, complete, and fail paths all tested.
app/src/screens/documents/selection/LogoConfirmationScreen.tsx Correctly fires SCAN_STARTED and SCAN_SUCCEEDED around launchKycVerification; serves as the reference pattern the fallback screens should mirror.
packages/mobile-sdk-alpha/src/constants/analytics.ts Adds OnboardingEvents group with 10 canonical event name constants; clean separation from existing diagnostic event groups.
packages/mobile-sdk-alpha/src/components/buttons/AbstractButton.tsx Adds trackEventRaw prop for verbatim event emission (bypassing Click: prefix); existing trackEvent behaviour unchanged.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant CP as CountryPickerScreen
    participant ID as IDSelectionScreen
    participant DC as DocumentCameraScreen
    participant NFC as DocumentNFCScanScreen
    participant RF as RegistrationFallback(MRZ/NFC)
    participant LC as LogoConfirmationScreen
    participant PM as provingMachine
    participant OF as onboardingFunnel.ts

    Note over OF: currentAttempt = null

    U->>CP: select country
    CP->>OF: trackOnboardingStep(COUNTRY_SELECTED)
    OF-->>OF: ensureAttempt → emit STARTED, then COUNTRY_SELECTED

    U->>ID: select document type
    ID->>OF: trackOnboardingStep(DOCUMENT_TYPE_SELECTED, branch)
    OF-->>OF: captureBranch → initialBranch locked

    U->>DC: enter camera scan
    DC->>OF: trackOnboardingStep(SCAN_STARTED, branch)

    alt Biometric success path
        U->>NFC: NFC scan succeeds
        NFC->>OF: trackOnboardingStep(SCAN_SUCCEEDED, branch)
        PM->>OF: trackOnboardingStep(PROOF_STARTED)
        Note over PM: didNewRegistrationProof=true
        PM->>OF: trackOnboardingStep(PROOF_SUCCEEDED)
        PM->>OF: completeOnboardingAttempt → COMPLETED
        OF-->>OF: currentAttempt = null
    else KYC fallback via LogoConfirmation
        U->>LC: tap No on chip check
        LC->>OF: setOnboardingBranch(kyc)
        LC->>OF: trackOnboardingStep(SCAN_SUCCEEDED, kyc)
        PM->>OF: completeOnboardingAttempt → COMPLETED
    else KYC fallback via RegistrationFallback
        U->>RF: tap Try a different method
        RF->>OF: setOnboardingBranch(kyc)
        Note over RF,OF: ⚠️ SCAN_SUCCEEDED never fired
        PM->>OF: completeOnboardingAttempt → COMPLETED
    else Proof failure
        PM->>OF: failOnboardingAttempt(proof_generation_started, reason)
        OF-->>OF: currentAttempt = null
    end
Loading

Comments Outside Diff (1)

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

    P1 Missing SCAN_SUCCEEDED for the KYC fallback path

    handleTryAlternative calls setOnboardingBranch('kyc') and awaits launchKycVerification(), but on success nothing fires SCAN_SUCCEEDED. LogoConfirmationScreen.handleNotFound follows the exact same KYC pattern and does fire it:

    trackOnboardingStep(selfClient, OnboardingEvents.SCAN_STARTED, { branch: 'kyc' });
    const result = await launchKycVerification(...);
    // ... on verified:
    trackOnboardingStep(selfClient, OnboardingEvents.SCAN_SUCCEEDED, { branch: 'kyc' });

    Without SCAN_SUCCEEDED, users who recover via the KYC fallback from this screen will always appear as dropped off at the scan_started step in the Mixpanel funnel — even though they go on to complete proof_generation_started → proof_generation_succeeded → completed.

    Note that SCAN_STARTED was already fired (for the biometric path in DocumentCameraScreen) and trackOnboardingStep dedupes, so a second SCAN_STARTED call here would be a no-op. Only SCAN_SUCCEEDED is missing. The same gap exists in RegistrationFallbackNFCScreen.handleTryAlternative (line 97–105 in that file).

Reviews (2): Last reviewed commit: "fix(analytics): emit Onboarding: Started..." | Re-trigger Greptile

Comment thread app/src/screens/onboarding/DisclaimerScreen.tsx Outdated
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