Repository navigation
Conversation
…nverting attempt correctly A funnel that repeats a step was satisfied by a single event, two rows in one millisecond converted in either order, and time between steps and the leavers list followed only a person's first attempt (and returned nothing when step 1 was a session step). Every funnel query now orders rows by one key (microseconds and Seq), reads times and leavers from the attempt the count is based on, and treats a mobile screen view as a page step. A scenario suite runs the compiled SQL on chDB against a TypeScript reference funnel over four example products, hand-built edge cases and random histories.
The Funnels tab gains a breakdown (where the session came from, or an event property the org sends) with a table of each group's conversion against everyone, a stat rail that compares the window with the one before and shows how long the funnel takes, and a steps table with the people dropped, p50 and p90 between steps and the top three things leavers did next. The timing query also answers the time since step 1, a breakdown can ask for up to twenty groups, and a product-event funnel draws its bars in one hue, sized to the panel.
This comment has been minimized.
This comment has been minimized.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const stepSubject = (step: FunnelStep): string => { | ||
| switch (step.kind) { | ||
| case "event": | ||
| return `event:${step.eventName}` | ||
| case "page": | ||
| return `page:${step.pagePath}` | ||
| case "session": |
There was a problem hiding this comment.
🔴 One page view counts as two steps
For an event step followed by a matching page step, stepsCanShareRow misses their overlap. windowFunnel then counts one view twice, inflating conversions while timing and leavers use distinct rows.
Learn more
The funnel uses windowFunnel only when no row can satisfy two steps; overlapping steps instead use walkQuery, which consumes each row once. An event step matches EventName regardless of Kind, while eventStepCondition also matches a page step on navigation or screen rows. The subject classifier assigns different prefixes to those steps, so it overlooks this overlap and sends their counts through windowFunnel. Timing and leavers always use the distinct-row walk, so their results can disagree with the counts.
Example: A single navigation row has EventName = '$pageview' and PagePath = '/pricing'. For steps event('$pageview') → page('/pricing'), the count reaches step 2 using that one row, although no second row exists.
Recommended fix: Make stepsCanShareRow detect cross-kind overlaps between event names and page steps, or use the distinct-row walk whenever an event and page step can match the same row. Add a chDB case with one matching navigation row and compare counts, timing, and leavers.
Was this helpful? React with 👍 or 👎 to provide feedback.
| : `median · 90% within ${fmtSpan(last.sinceStartP90Ms)}`, | ||
| }), | ||
| statRailItem(h, { | ||
| eyebrow: "Biggest drop", |
There was a problem hiding this comment.
🟡 Empty conversions display a timing claim
When nobody converts, stats receives zero-valued timing and displays median · 90% within –. The timing query returns zeroes for empty steps, so the intended nobody converted message never appears.
(Refers to this code)
Learn more
The timing query emits a zero-valued timing row even when nobody reached a step. Those zeroes are carried into the funnel row through funnelStepDetails. The display checks for an absent value rather than checking whether anyone converted, so the no-conversion case renders a statement about the 90th percentile with no duration.
Example: For signup_started → signup_completed, 12 people enter and zero complete. The tile shows median · 90% within – instead of nobody converted.
Recommended fix: Gate the final-step timing value and subline on converted > 0, not only on whether the timing fields are present. Keep the existing one-step state separate.
Was this helpful? React with 👍 or 👎 to provide feedback.
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
| subline: | ||
| last?.sinceStartP90Ms === undefined | ||
| ? hasSteps | ||
| ? "nobody converted" | ||
| : "one step" | ||
| : `median · 90% within ${fmtSpan(last.sinceStartP90Ms)}`, |
There was a problem hiding this comment.
Warning
Time to convert shows “median · 90% within –” when nobody converted
F1 · Warning · correctness
A step nobody reached still answers a timing row of zeros (productEventsFunnelTimingRowSchema: "a step nobody reached answers 0, never a missing row"), so last.sinceStartP90Ms is 0, never undefined. On a funnel with no conversions the tile renders value “–” and subline “median · 90% within –”, and the “nobody converted” copy never renders.
Decide the state from the last step's count (`converted === 0`) instead of testing `sinceStartP90Ms === undefined`, which the query never sends.
| subline: | |
| last?.sinceStartP90Ms === undefined | |
| ? hasSteps | |
| ? "nobody converted" | |
| : "one step" | |
| : `median · 90% within ${fmtSpan(last.sinceStartP90Ms)}`, | |
| subline: !hasSteps | |
| ? "one step" | |
| : converted === 0 | |
| ? "nobody converted" | |
| : `median · 90% within ${fmtSpan(last?.sinceStartP90Ms ?? 0)}`, |
🤖 Prompt to fix with an AI agent
In `apps/web/src/page/product-analytics/view-funnels.ts:300-305`: `Time to convert` shows “median · 90% within –” when nobody converted.
A step nobody reached still answers a timing row of zeros (`productEventsFunnelTimingRowSchema`: "a step nobody reached answers 0, never a missing row"), so `last.sinceStartP90Ms` is `0`, never `undefined`. On a funnel with no conversions the tile renders value “–” and subline “median · 90% within –”, and the “nobody converted” copy never renders.
Replace those lines with:
subline: !hasSteps
? "one step"
: converted === 0
? "nobody converted"
: `median · 90% within ${fmtSpan(last?.sinceStartP90Ms ?? 0)}`,
Verify the problem exists at that location before changing it, and keep the fix to those lines.
…tics "Filter by property" on the Steps panel shows a where line under each event step (plan = "pro"). It runs on Enter or once typing pauses, a clause that does not parse yet keeps the filter that is running, and turning it off clears the filters. The line moves into the step builder kit, so the dashboard editor and the page draw the same one. A one-row where editor is one line tall again: its min-height went to the wrapper instead of the textarea, so it kept the two-row minimum. A filtered step's label names its filter.
Sessions carry a UTM medium and campaign, where a visitor came from tilts how far they get, and signup_completed, onboarding_connected and plan_started carry a property. A preview or local stack now has something to break a funnel down by and to filter a step on.
How to build a funnel on Product analytics, what Count by and the window mean, how to read the result, breakdowns, step filters, repeated steps, and server and mobile steps.
…h no events Review findings on the funnel queries: - Rows with the same millisecond and Seq (two server events from one request) were converted by the count but not by the walk, and made time between steps negative. Tied rows now sort in step order everywhere, and the chain walks by position, so the count, the times and the leavers agree. - An event step on $pageview or $screen beside a page step let one view count for both. Those funnels now take the walk. - A visitor whose session matched step 1 and who sent no event was missing from the leavers. They count under nothing after. - The order key is milliseconds and Seq modulo 2^20, so a long session no longer reverses two rows across Seq 1024. - A window longer than 366 days is refused instead of overflowing.
Maple review🟡 Confidence 6/10 · needs attention. Held at 6 because a correctness warning is open. Adds a breakdown table, previous-window deltas, time to convert and a fuller steps table to the Funnels tab, plus a per-person walk that replaces
Still open from earlier reviews
|
The Funnels tab on Product analytics answered "how many reached each step". This makes it answer the questions that come next: which channel converts, how long it takes, is it getting better, and where the people who left went.
Under it: the timing query also answers the time since step 1, and a breakdown can ask for more than the widget's six groups (
breakdownLimit, 1 to 20).Screenshots
Break down by UTM source (dark, light)
Drop-off view: before, after
Bars view: before, after
Dashboard funnel tiles (query-set funnels keep their colours, bars fill the tile)
Verification
bunx vitest run src/page/product-analytics src/page/dashboards src/page/kit src/labinapps/web: 852 pass. New scene tests cover:apps/cli/test/funnel-scenarios.test.ts(chDB): 79 pass, with the time since step 1 checked against the reference on every example funnel and on random histories.packages/query-engine814 pass,packages/query-model,packages/web-data,packages/backenddashboards andapps/apiv2 dashboards tests pass.web,views,query-engine,query-model,domain,backend,web-data,cli.oxlint(both configs),ui:index:check,ui:patterns:checkand the UI baseline are clean./lab/product-analytics?state=funnels-breakdown,funnels,funnels-barsand/lab/widgets), dark and light.Not done yet: a preview run on real data.
Stack: #2171 → #2172 → this PR. The diff also shows #2584's commit until that merges.
🤖 Generated with Claude Code
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.