feat(insights): add transforms for horizontal funnel bar chart#60453
Merged
sampennington merged 1 commit intoMay 28, 2026
Conversation
sampennington
added a commit
that referenced
this pull request
May 28, 2026
Render layer for the horizontal funnel bar chart, stacked on #60453 (data layer). One BarChart with N stacked bands; StepDecorations overlays the ordinal glyph column, the per-step header (with FunnelStepMore + DuplicateStepIndicator + average-time), and the ValueInspector row above and below each band. Wired into Funnel.tsx behind the existing PRODUCT_ANALYTICS_HOG_CHARTS_FUNNEL feature flag; legacy FunnelBarHorizontal remains the default. Generated-By: PostHog Code Task-Id: 3ac7029f-8133-4274-b3b3-80e549e1d1be
Contributor
|
Size Change: 0 B Total Size: 80.6 MB ℹ️ View Unchanged
|
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
products/product_analytics/frontend/insights/funnels/FunnelBarHorizontalChart/funnelBarHorizontalTransforms.ts:50
The `labels` array is populated with opaque numeric-index strings (`"0"`, `"1"`, `"2"`, …). The hog-charts `TooltipContext.label` field flows directly from this array, so any tooltip or axis renderer that consumes `label` will receive an unhelpful index string rather than the step name. The step name is already available as `step.name`, and a `getLabel`-style option already exists in `BuildOptions` — passing it through for the axis labels as well would make the data layer self-contained without any extra cost.
```suggestion
const labels = steps.map((step) => step.name)
```
### Issue 2 of 2
products/product_analytics/frontend/insights/funnels/FunnelBarHorizontalChart/funnelBarHorizontalTransforms.ts:74
The `?? steps[0]` fallback on `representative` is unreachable. `buildBreakdownSeries` is only entered when `isBreakdownLayout` passes (i.e. `steps[0].nested_breakdown.length > 1`), and `breakdownIndex` is always in the range `[0, breakdownCount)` where `breakdownCount = steps[0].nested_breakdown!.length`. Under the "no superfluous parts" rule, this dead branch can be removed.
```suggestion
const representative = variantAtStep(steps[0], breakdownIndex)!
```
Reviews (1): Last reviewed commit: "Merge branch 'master' into posthog-code/..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![hex-security-app[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
sampennington
force-pushed
the
posthog-code/funnel-bar-horizontal-transforms
branch
from
5ef8ca3 to
d9f6faa
Compare
Pure data layer for the horizontal funnel bar chart adapter. buildFunnelBarHorizontalData turns visibleStepsWithConversionMetrics into the hog-charts series + labels shape — one chart, N stacked bands (one per step), with a transparent filler series that anchors each band to 100%. Render layer (StepDecorations, FunnelBarHorizontalChart, tooltip, stories, Funnel.tsx wiring) follows in a stacked PR. Generated-By: PostHog Code Task-Id: 3ac7029f-8133-4274-b3b3-80e549e1d1be
sampennington
added a commit
that referenced
this pull request
May 28, 2026
Render layer for the horizontal funnel bar chart, stacked on #60453 (data layer). One BarChart with N stacked bands; StepDecorations overlays the ordinal glyph column, the per-step header (with FunnelStepMore + DuplicateStepIndicator + average-time), and the ValueInspector row above and below each band. Wired into Funnel.tsx behind the existing PRODUCT_ANALYTICS_HOG_CHARTS_FUNNEL feature flag; legacy FunnelBarHorizontal remains the default. Generated-By: PostHog Code Task-Id: 3ac7029f-8133-4274-b3b3-80e549e1d1be
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
products/product_analytics/frontend/insights/funnels/FunnelBarHorizontalChart/funnelBarHorizontalTransforms.test.ts:215-235
**Test name promises breakdown coverage it doesn't deliver**
`isBreakdownLayout` checks `steps[0].nested_breakdown.length > 1`, but `steps[0]` here has no `nested_breakdown` at all, so the function returns `false` and the test runs entirely through `buildSingleSeries`. The inline comment even acknowledges this explicitly. As a result, `buildBreakdownSeries` with `FunnelStepReference.previous` — where `basisCount` varies per-step via `getReferenceStep` — has no test coverage at all. A fixture where both `steps[0]` and `steps[1]` carry at least two `nested_breakdown` variants, with `fromBasisStep` and `count` set to diverge under `previous` vs `total` reference, would actually exercise that path.
Reviews (2): Last reviewed commit: "feat(insights): add transforms for horiz..." | Re-trigger Greptile |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
New frontend-only transform utility with comprehensive tests; no existing files modified, no backend/API/migration changes. Both resolved bot comments were addressed in the current diff. The hex-security comment references a file not in this PR and is not applicable.
sampennington
added a commit
that referenced
this pull request
May 28, 2026
Render layer for the horizontal funnel bar chart, stacked on #60453 (data layer). One BarChart with N stacked bands; StepDecorations overlays the ordinal glyph column, the per-step header (with FunnelStepMore + DuplicateStepIndicator + average-time), and the ValueInspector row above and below each band. Wired into Funnel.tsx behind the existing PRODUCT_ANALYTICS_HOG_CHARTS_FUNNEL feature flag; legacy FunnelBarHorizontal remains the default. Generated-By: PostHog Code Task-Id: 3ac7029f-8133-4274-b3b3-80e549e1d1be
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
Splitting the horizontal funnel bar chart adapter (#60385) into reviewable chunks. This PR is the pure data layer; the render layer follows in a stacked PR.
Changes
buildFunnelBarHorizontalData(steps, options)turns the funnel'svisibleStepsWithConversionMetricsinto the hog-chartsseries+labelsshape — one chart with N stacked bands (one per step), where each band's bars are anchored to 100% via a transparent filler series. ReusesgetReferenceStep,getStepBreakdownSeries, andgetFunnelsColorfrom the existing funnel utilities so behavior matches the legacy chart.No rendering, no wiring, no flag yet. 17 parameterized tests cover no/single/multi-step funnels, breakdown variants (including collapsing-to-a-single-visible-breakdown), `total` vs `previous` reference, drop-off filler placement, and zero-count edge cases.
🤖 Agent context
Pure data extraction from the inlined adapter in PR #60385, so reviewers can land the math independently of the React layer.
How did you test this code?