Better wallet-selection analytics - #591
Open
mytonwalletorg wants to merge 12 commits into
Open
Conversation
Four call sites had a traceId in scope and did not pass it, so
AnalyticsManager backfilled each event with a freshly minted UUIDv7
(analytics-manager.ts:116), leaving well-formed but unjoinable traces.
The one that matters is openModal(). It mints a traceId, hands it to
`modal.open({ traceId })`, and then passes `options?.traceId` — not the
local one — to trackWalletModalOpened. A dApp calling openModal() with no
arguments, which is the normal case, therefore reports a modal-opened
event on a private trace while the modal it describes runs on another.
That event is the sole source of `connection-started` in the analytics
backend, and measured trace overlap between it and every other event is
0%. This is a one-word fix for that.
The other three (wallets-modal-manager.ts:154, ton-connect-ui.ts:932 and
:952) are the same defect in trackConnectionStarted and
trackConnectionCompleted. Worth knowing that these are visible only to
dApps subscribing to the `ton-connect-ui-*` window events: the analytics
adapter listens for the core-prefixed `ton-connect-connection-*`, so the
UI's copies of those two events reach no backend. They are a public-API
fix, not a reporting one.
The traceId parameter is optional on every one of these event factories,
so all four omissions type-checked silently.
Purely additive: no event added, removed or changed in shape, and no row
counts change. A field that held a random value now holds the real one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`openSingleWalletModal()` had no trace anywhere: the manager emitted `connection-started` with no traceId, and the connect calls it drives passed none either, so AnalyticsManager minted a separate UUIDv7 for each event (analytics-manager.ts:116) and nothing from this flow could be correlated. Unlike the dropped arguments fixed in the previous commit, there was no traceId in scope to pass — this flow never had one. Mints one in `SingleWalletModalManager.open()` and carries it to the tracker, to the embedded connect call, and — via the modal state — into the connection modal that issues the remote connect. Mirrors how `WalletsModalState` already wraps its union in `OptionalTraceable`. This matters beyond tidiness: deep-linking a named wallet is one of the flows currently pooled into the `explicit_wallet` bucket, and until it carries a shared trace it cannot be told apart from the other two. Touches public surface, which is why it is separate from the argument fixes: `SingleWalletModalState` gains an optional `traceId`, and the manager's `openSingleWalletModal(wallet, options?)` gains an optional parameter. Both are backward compatible — dApps subscribing via `onSingleWalletModalStateChange` see one additional optional field — and the public `TonConnectUI.openSingleWalletModal(wallet: string)` signature is unchanged. No event is added, removed or altered in shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`connection-selected-wallet` is emitted only by `@tonconnect/ui`
(TonConnectUITracker.trackSelectedWallet), so a dApp using bare
`@tonconnect/sdk`, its own wallet picker, or a deep link produces no
selection row at all — invisible to the channel table, size unknown.
Core `connect()` already receives the wallet as an argument, so the
channel is derivable from the argument shape — a fact rather than the
ambient UI state the existing event reconstructs itself from. Classifies
it and emits it beside the existing events, where the traceId is already
minted.
Values are prefixed by transport so that axis survives a flat enum:
js-embedded dApp runs inside that wallet's own browser
js-injected wallet exposes a JS bridge, typically an extension
http-specific-wallet one named wallet, its own bridge and universal link
http-any-wallet the universal `tc://` link, no wallet chosen yet
wallet-connect the WalletConnect transport
`js-%` and `http-%` therefore recover the transport without an IN-list.
The suffix carries what differs within a transport, which is a different
question per transport: for the JS bridge, whether the page is inside the
wallet's browser; for HTTP, whether one wallet was targeted. Those share
the `{jsBridgeKey}` and bridge shapes respectively, so neither split is
visible in the argument alone —
`InjectedProvider.isInsideWalletBrowser` supplies the first,
`Array.isArray` the second.
`js-embedded` matters most: it is one of the populations currently pooled
behind `wallets_menu: explicit_wallet`, and `custom_data.provider` on
connection-completed is only `'http' | 'injected'`, which cannot express
it.
Dual emission, not replacement: nothing existing changes shape or volume,
so dashboards keep working across the long tail of unupgraded dApps, and
during the overlap a client emits both on one trace, making the
correction factor measurable rather than assumed.
Known gap, deliberate: `client_environment` (miniapp/web) is empty for
dApps that supply no environment, i.e. exactly the SDK-only population
this reveals — `DefaultEnvironment.getClientEnvironment()` returns ''.
Read it as unknown, not as web. Unifying detection belongs with the UI
commit that follows.
Counts connect initiations, not user actions: one journey legitimately
produces several rows on one trace with different kinds. A useful
consequence — restores never call connect(), so a connection-completed
with a trace-matched connection-initiated is a new connection and one
without is a restore, a split the warehouse cannot currently make.
Tests cover the ways this fails silently: the classification (both
`{jsBridgeKey}` branches, which are structurally identical), the adapter
listening on the core prefix rather than the ui prefix (both type-check;
the wrong one never fires), the tracker's real dispatch to a window
event, and the pascalToKebab round-trip that names the wire event.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`connection-completed` fires on every session restore, not only on new connections. Each provider's restoreConnection() replays the stored connect event to its listeners — bridge-provider.ts:248, injected- provider.ts:147, and the wallet-connect provider via onConnect — which reaches onWalletConnected and emits a completion indistinguishable from a fresh one. TonConnectUI calls restoreConnection() automatically on construction, so this fires on every page load for an already-connected user. A restore is a real connection event: a live session is established and the dApp can transact, and inside a wallet browser the auto-connect stands in for a Connect tap the user would otherwise have made. The problem is not that restores are noise — it is that completions silently mix two different bases: is_restore: false a relationship was formed once per relationship is_restore: true an existing one was exercised once per visit Summed, they answer neither question: acquisition is diluted by return visits, and the total scales with page views. Split, the second half is the only return signal available, and worth keeping rather than discarding. Marks the replayed event with `restored` and carries it through to an `is_restore` boolean on connection-completed. Chosen over forwarding the existing `connection-restoring-completed`, which is emitted (ton-connect.ts:559) but has no adapter listener: that would add a row per restore, and restores appear to outnumber fresh connections. Trace correlation cannot answer this instead. A completion's trace comes from the wallet, not from us — BridgeGateway reads `trace_id` off the incoming SSE payload (bridge-gateway.ts:254) and BridgeProvider mints a fresh one when it is absent (:447). So a fresh connection only shares a trace with its connect() when that particular wallet echoes trace_id back through the bridge, which nothing in this SDK enforces. Matching completions against connection-initiated would classify every non-propagating wallet's connections as restores. `is_restore` is always present rather than set only when true, so the column can be filtered without treating absent as either case. Adds a field to an existing event; no event added or removed, no row counts change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`connection-selected-wallet` is reconstructed after the fact from three module-level Solid signals by two watchers in WalletsModalManager's constructor (wallets-modal-manager.ts:92-143). Everything wrong with it follows from that: `explicit_wallet` is a signal's initial value rather than a recorded fact, a page-load replay emits it with no user action, one tap emits twice, and a dedup guard silently drops real picks. These two events replace it, emitted directly from the click handler with every field supplied by the call site. Nothing is inferred from ambient state and `trace_id` is required rather than optional, so the next dropped trace is a compile error instead of an orphaned row. Two events because mobile and desktop differ in how many decisions the user makes. On mobile the connection modal redirects to the wallet on mount (mobile-connection-modal.tsx:214), so the tap is the commitment. On desktop it opens a second screen offering QR, extension and desktop app (desktop-connection-modal.tsx:204-210), so the tap only narrows the choice: mobile tile wallet-selected manual desktop tile wallet-preselected manual desktop footer button wallet-selected manual openSingleWalletModal, mobile wallet-selected auto-dapp-directed openSingleWalletModal, desktop wallet-preselected auto-dapp-directed embedded auto-connect wallet-selected auto-embedded A preselection with no matching selection on the same trace means the user scanned the second screen's default QR; the wallet identity is not lost, because the preselection carries it. `connection_mode` records which transport was picked on the desktop screen — the second decision, which nothing captured before. connection-initiated sees the Extension click as js-injected but usually nothing for Desktop, since onClickDesktop only regenerates the link when the previous mode was extension. The footer emissions are wired into the buttons' onClick rather than into onClickMobile/onClickDesktop/onClickExtension, because the mount-time dispatch calls those same handlers with no user involved — emitting from inside them would report a selection nobody made. Nothing is emitted for: the first-screen QR on either platform, which is the multi-bridge tc:// link any wallet can answer, so no wallet was chosen here at all (already recorded as http-any-wallet); session restores, covered by is_restore; the WalletConnect tile, which bypasses onSelect and has no traceId in scope, and emits nothing today either. The tracker reaches views through appState, the same channel already used for the connector, so no public API changes. Dual emission: the existing watchers and `selected-wallet` are untouched, so v1 dashboards keep working while adoption builds. Note the old event counts embedded auto-connect through the module-global signal write and the new ones mark it auto-embedded, so raw v1/v2 counts will differ by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule deciding whether a pick is a preselection or a selection was duplicated in wallets-modal.tsx and single-wallet-modal-manager.ts, and was the one piece of the wiring that could not be verified in a browser: it keys off `isMobile`, which derives from window.innerWidth, and the automation viewport is pinned regardless of window size. Moves it to one helper taking `isMobile` as a parameter, so both call sites share the rule and both branches are covered by a unit test. Getting this backwards would otherwise be invisible — both events exist, both are accepted downstream, and the modal behaves identically either way. No behaviour change. The desktop branch was verified end to end in the dev app before and after this refactor: one wallet-preselected per tile click on both universal-modal and all-wallets-list surfaces, silence on the connection screen's mount-time dispatch, and one wallet-selected with connection_mode on a footer click. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`connection-initiated` fired on every connect(), including the two calls that exist only to render something. The desktop universal modal builds a multi-bridge QR as it renders, and the desktop connection screen rebuilds a wallet-specific one in an effect that can re-run. Both open real sessions, so downstream they were indistinguishable from a connect the user asked for — measured: opening the modal once produced two `connection-initiated` rows before any click. Worse, the two were indistinguishable from each other by kind: the desktop mount effect emits http-specific-wallet with no user involved, and a mobile tile tap emits http-specific-wallet because the user chose it. Same value, same trace, opposite meaning. Adds `linkDisplayOnly` to the connect options, set at those two sites, so core reports `connection-link-generated` instead. A separate event rather than a flag on the existing one: a flag has to be remembered in every query, and forgetting it silently reinstates the problem — the same shape of defect this whole change set exists to remove. The new event carries the same source classification, so QR displays can still be counted per wallet or bridge without inflating initiations. The default is the honest reading for anyone who does not set it: a bare connect() really is an initiation, so dApps and every unflagged call site are unaffected. Verified against a local collector: opening the modal now yields connection-link-generated (http-any-wallet) twice and no connection-initiated; picking a wallet yields wallet-preselected plus connection-link-generated (http-specific-wallet). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Analytics could not be checked in development at all. The hosted collector answers 400 to every request from a localhost origin — verified against batches containing only long-established event names, so it is the origin and not the payload — and `handleClientError` logs and drops the batch without retrying. Nothing surfaces beyond a console line, so an analytics regression would go unnoticed until release. `AnalyticsManager` already accepted an `analyticsUrl`, but `initAnalytics` never passed one, leaving no way to redirect events. Threads it through `AnalyticsSettings.url`, and adds a local sink plus a query parameter on the dev harness: node packages/ui/tools/analytics-sink.mjs open http://localhost:3000/?analyticsUrl=http://localhost:3100/events The sink prints one line per event with the fields this work introduced — surface, selection_source, connection_mode, connection_source_kind, is_restore — and a per-name tally on exit. A query parameter rather than an env var because Vite does not expose shell VITE_* vars here, and because it needs no restart to toggle. This is what verified the preceding commits end to end: the events reach a real collector over a real POST with real batching, rather than only being observed as window events. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The WalletConnect entry in the all-wallets list connects directly rather than going through `onSelect`, so it bypassed both the new events and the old one — picking it produced no selection row at all, before or after this change set. It also passed no traceId, so the connect it started was unjoinable. `AllWalletsListModal` was the one selection surface receiving no modal state, so it now takes the traceId and reports the pick itself. Recorded as a selection on both platforms rather than following the platform rule, because WalletConnect hands off to its own picker immediately: our connection screen, the thing that makes a desktop pick merely a preselection, never renders for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`client_environment` distinguishes a Mini App from a web page, and was answered only by `@tonconnect/ui`: core's DefaultEnvironment returned an empty string for both. So every dApp on the bare SDK reported no environment — and that is precisely the population `connection-initiated` was added to make visible, which would have arrived untyped. Moves the detection into core, and has the UI re-export it, so there is one implementation rather than two definitions of the same question. Core now also answers getTelegramUser() for the same reason. Computed on first use rather than at import: resolving it reads location.hash and both reads and writes sessionStorage, which should not happen merely because someone imported the SDK. Two things worth knowing for anyone touching this again: - The `tapps/launchParams` value is a JSON-encoded string in some Telegram clients and a bare query string in others, so a JSON.parse failure means "already raw", not "unusable". A first pass at this dropped the bare form, which the UI's existing tests caught. - The Window augmentation is declared in both packages because api-extractor does not carry `declare global` into the rolled-up types, so the UI's messaging half cannot see core's. The UI keeps the Telegram messaging half — postEvent, sendExpand, sendOpenTelegramLink — which is UI-specific and stays put. Its existing test suite passes unchanged against the shared implementation, which is the evidence that the move is behaviour-preserving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The analytics backend already stores the wallets-list bridge key as `bridge_key` — the injected provider's own events use it — so the new connection events reported the same value under a second name. Renames `js_bridge_key` to `bridge_key` on connection-initiated and connection-link-generated, so ingesting them needs no new column for it and queries do not have to know which event they are reading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by an adversarial review of this branch. Two Telegram buttons committed to a wallet without reporting it, so @wallet was under-counted while every other wallet was recorded: - mobile-universal-modal onSelectTelegram. The wallet row on that screen explicitly excludes @wallet, so this CTA is the only route to it there, and it redirects — a commitment. It now emits wallet-selected on surface universal-modal. - desktop-connection-modal. For @wallet the whole instrumented footer is replaced by a single Telegram button, so none of the trackSelected wrappers rendered. It now reports the desktop route like the others. Both previously produced connection-initiated with no selection on the trace, which the event's own documentation tells consumers to read as "the user scanned the default QR". The TMA move was also not behaviour-preserving, in two ways the existing suite did not cover: - Platform resolution used `??` where the original used a truthiness check, so an empty tgWebAppPlatform was kept as a platform. isInTMA only compares against 'unknown', so an empty launch param would have reported being inside a Mini App. - webAppVersion gained a window.Telegram.WebApp.version fallback the original could never reach: it was guarded by `if (!webAppVersion)` after the variable was seeded with '6.0'. That feeds versionAtLeast(), which decides how sendOpenTelegramLink opens links — a UX path, not just analytics. The dead branch is reproduced deliberately; fixing it is a separate change. Both are now pinned by tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
3 participants
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.
The problem
We cannot see how users pick a wallet.
connection-selected-wallethas awallets_menufield. Over 90 days, 93% of rows sayexplicit_wallet. That reads as "almost nobody sees the wallet list".That is not what happens. No code ever sets
explicit_wallet. It is the starting value of a variable in the UI, and it is what the event carries when no wallet list has been drawn yet. It means "we recorded nothing", not "the user chose directly".The deeper issue is how the event is built. It is not sent when a user clicks. It is rebuilt afterwards from three shared variables, by two watchers. Because of that:
localStorageon page load, so every returning visitor sends an event without touching anything.So the number mostly counts page views, not choices.
What this adds
The old event is not changed. It keeps working while dApps upgrade, which takes months. New events are added alongside it.
wallet-preselectedwallet-selectedconnection-initiatedconnection-link-generatedEach is sent at the moment the user acts and carries its own values. Nothing is guessed from shared state.
Why two events for picking a wallet. Mobile and desktop differ. On mobile, tapping a wallet takes you straight to the wallet app — one decision. On desktop, clicking a wallet opens a second screen with QR, extension and desktop options — two decisions. One event could not describe both honestly.
New fields
surface— where the pick happenedselection_source—manual, orauto-embedded/auto-dapp-directedwhen nobody clickedconnection_mode— which option was chosen on the desktop screenconnection_source_kind— how the connect reaches the walletis_restore— marks connections that are a restored session, not a new oneWhat this fixes
openModal(), is whyconnection-startednever links to anything.@walletwas under-counted while every other wallet was recorded.client_environment. The collector rejects that value, so all of their events were being thrown away. Mini App detection moved into core, so they now reportweborminiapp.What changes for data users
connection-completedcounts restores. It always did.is_restorenow separates them. A restore is a real connection, but it happens on every visit, while a new connection happens once. Adding them together answers neither question.connection-initiatedfires on every connect, and bare-SDK dApps stop being rejected.Before merging
ton-connect/analyticsbranchfeat/dapp-sdk-wallet-selection-events.is_restoreis a required field on an exported type.Testing
310 tests pass (215 sdk, 95 ui). Lint, types and builds clean.
The desktop flow was clicked through in a browser against a real collector: one
wallet-preselectedper click, nothing on mount, onewallet-selectedwith the right option, and the old event still firing beside it.The mobile branch could not be clicked through, because the test browser locks the window width. It is covered by a unit test instead.
This branch was then reviewed by a separate pass, which found two real bugs that are fixed here: the two Telegram buttons above, and two places where the Mini App detection was not identical to the code it replaced.