fix(desktop): record an onboarding step view once the step is settled - #89594
Draft
adboio wants to merge 2 commits into
Draft
fix(desktop): record an onboarding step view once the step is settled#89594adboio wants to merge 2 commits into
adboio wants to merge 2 commits into
Conversation
`Onboarding step viewed` fired from a mount effect that ran before the lookups choosing the step set had answered, so every person recorded a `project-select` view including those with one project, whose step was then dropped. The self-heal that moves a person off a dropped step emitted nothing, so anyone it moved into `consent` advanced with no matching view. Record the view from one effect, once no pending lookup can still drop the step. Reset a persisted step id from a retired set at rehydration, which the old flow could otherwise show as an empty card. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
1 task
Contributor
|
React Doctor found 6 issues in 1 file · 6 warnings. 6 warnings
Reviewed by React Doctor for commit |
Contributor
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. |
HostHog preview —
|
Contributor
🦔 ReviewHog reviewed this pull requestFound 0 must fix, 2 should fix, 0 consider. Published 2 findings (view the review). |
Contributor
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Two paths could still emit `Onboarding step completed` for a step that recorded no view. A failed GitHub integrations lookup left its gate unanswered for the session, holding `install-cli` pending while the step rendered and its Continue button worked. A person could also leave any pending step before its gate answered, through the button or the right-arrow hotkey. Read the query's pending state rather than its data, so a failed lookup answers the gate and keeps the step. Record the view from `handleNext` and `handleComplete` as well as the effect, so a completion never precedes one. Track step entry separately from the view, so a delayed view no longer measures `duration_seconds` from the previous step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
The desktop onboarding funnel reports two steps wrong, so the canvas the team reads gives a false picture of where people drop.
project-selectrecords 439 people viewing and 122 advancing over 3 days for non-staff, against roughly 98% on every other step.consentrecords 110 viewing and 171 advancing, which is more completions than views.consentadvanced with no matching view.welcomestep id from an older build reaches analytics the same way, which is where the report behind this PR started.Changes
stepGatePendinginpackages/core/src/onboarding/steps.tsanswers that per step, so a step no lookup can drop is never held.handleNextandhandleBackno longer record views. They record the completion and move the step, and the effect follows.handleNextpreviously recorded the next step's view beforenext()ran, which named the wrong step whenever the step set shifted in between. That path is gone.handleNextandhandleCompleterecord the view first, which covers a person who leaves a step before its gate answers, through the button or the right-arrow hotkey.dataundefined, which would otherwise holdinstall-clipending for the rest of the session.duration_secondsfrom the previous step.invite-codemapping stays ahead of that reset so a person mid-flow still moves forward toconsent.migrate.computeActiveStepsoptions object becomes a namedStepGatestype. Mechanical.Note
Step view counts change the day this ships.
project-selectviews drop by roughly the number of people who hold one project.consentviews rise to meet its completions. Treat pre-merge and post-merge funnel numbers as separate series.How did you test this code?
stepGatePendingcases topackages/core/src/onboarding/steps.test.ts. They catch a step wired to the wrong lookup, or a lookup added tocomputeActiveStepsand forgotten here, which would record a view for a step that is then dropped.packages/ui/src/features/onboarding/onboardingStore.test.tswith retired-step and current-step cases. They catch a migration that drops theinvite-codemapping, and one that resets a valid step.@posthog/coreand@posthog/uivitest suites locally.useOnboardingFlowand would need a component render with mocked query sources. Poor trade at that level; worth a reviewer's eye.Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written with Claude Code (Opus 5). Skills invoked:
/writing-tests,/writing-code-comments,/writing-pr-descriptions.The work started from a self-driving inbox report about a single retired
welcomestep id. Investigating why that report cleared the scout's bar surfaced the two funnel numbers above, which share one cause with it.This replaces #89543, which grew a spinner, a global wait on every lookup, and a timeout to bound that wait. Each of those existed only to contain the previous one, and the spinner made everyone wait on
gh auth status, a network call that cannot affect which step comes first. Waiting per step removes all of it. #89463 was closed earlier as superseded; its rehydration guard is carried here.ReviewHog found both error paths after the first push: the consent gate was fixed and the integrations gate was not, and a pending step could still be completed through the hotkey. Recording the view from the navigation handlers closes that for every step and every path, rather than disabling a button with no feedback.
The funnel numbers in Problem come from queries against project 2 in this session. No customer data, session content, or internal material is quoted anywhere in this PR.