fix(funnel): stop the breakdown limit from dropping lower funnel steps

[ayushjhanwar-png]

The bug

When a funnel has a breakdown and more distinct breakdown values than limit, every step of every breakdown row reports an identical count at 100% — the funnel looks like it converts perfectly end to end.

Observed on a report with 185 distinct paths against a limit of 50:

/script-to-video      128 100%   128 100%   128 100%   128 100%   128 100%

The underlying rows for that path:

by_level   1:67  2:105  3:1  4:22  5:128
correct    323 → 256 → 151 → 150 → 128     100% / 79.3% / 46.7% / 46.4% / 39.6%

128 is the deepest-level count, repeated across every step.

Cause

toSeries bails out of the entire reduce once the accumulator holds limit keys:

const series = funnel.reduce((acc, f) => {
    if (limit && Object.keys(acc).length >= limit) {
      return acc;                    // drops EVERY remaining row
    }
    const key = breakdowns.map((b, i) => normalizeBreakdownValue(f[`b_${i}`])).join('|');
    if (!acc[key]) acc[key] = [];
    acc[key].push({ level: f.level, count: f.count, ... });
    return acc;
}, {});

The funnel query ends with ORDER BY level DESC, so rows arrive grouped by depth — all max-level rows first, then the next level down, and so on. The limit is therefore reached while only max-level rows have been seen, and every subsequent row is discarded — including the lower-level rows belonging to series already accepted.

Each series is left holding one row. fillFunnel accumulates bottom-up, so that single count propagates into every step, and totalSessions (taken from the level-1 entry) becomes equal to it — making every percent exactly 100%:

fillFunnel([{ level: 5, count: 128 }], 5)
  filled     [1:0,   2:0,   3:0,   4:0,   5:128]
  accumulate [1:128, 2:128, 3:128, 4:128, 5:128]
  totalSessions = 128  →  percent = 100% at every step

Reports with fewer breakdown values than their limit are unaffected, which is why this only shows up once a breakdown dimension grows past the limit.

Fix

The limit caps how many series are returned, so it should only reject keys that are new:

const key = breakdowns.map((b, i) => normalizeBreakdownValue(f[`b_${i}`])).join('|');
if (!acc[key]) {
  if (limit && Object.keys(acc).length >= limit) return acc;
  acc[key] = [];
}
acc[key].push(...);

The number of series returned is unchanged — still capped at limit.

Verification

Simulated against this branch's toSeries (including normalizeBreakdownValue) and fillFunnel, with rows in the exact level DESC order the query produces — 3 breakdown values, limit: 2:

BEFORE | series: 2 | rows in series[0]: 1
       steps: 128 -> 128 -> 128 -> 128 -> 128
    percents: 100.0% 100.0% 100.0% 100.0% 100.0%

AFTER  | series: 2 | rows in series[0]: 5
       steps: 323 -> 256 -> 151 -> 150 -> 128
    percents: 100.0% 79.3% 46.7% 46.4% 39.6%

Series count stays capped at 2 in both cases.

The same change was verified end to end on a downstream deployment against live data: the corrected numbers match the ClickHouse rows exactly.

Tests

Adds packages/db/src/services/funnel.service.test.ts covering:

• every level of a series is kept once the limit is reached
• the number of series is still capped at the limit
• a limited series is not flattened into an all-100% funnel
• the no-breakdown path still returns a single series

Two of the four fail without this change. Note: these were executed against a fork whose toSeries is identical to this file's; I wasn't able to install this repo's dependencies locally to run the suite here, so CI is the real check on them.

Also worth a look (not changed here)

Which series survive the limit is "first limit keys encountered in level DESC order" — biased toward series that converted deepest rather than the largest. Results are sorted by total afterwards, so ordering looks right, but a high-traffic breakdown value that never converts deeply can still be dropped. Selecting the top-N by total before slicing would be more predictable.

Summary by CodeRabbit

• Bug Fixes
  • Fixed funnel breakdown limits so retained series continue to include all funnel levels.
  • Prevented limited breakdowns from incorrectly appearing as flattened 100% series.
  • Ensured empty breakdowns still produce a single series.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ceea720c-83d7-4471-9874-1316d3d44759

📥 Commits

Reviewing files that changed from the base of the PR and between 3060ca1 and 64b435c.

📒 Files selected for processing (2)

• packages/db/src/services/funnel.service.test.ts
• packages/db/src/services/funnel.service.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

FunnelService.toSeries now limits new breakdown series without discarding later funnel levels for accepted series. New tests cover limits, level preservation, regression behavior, and empty breakdowns.

Changes

Funnel series limiting

Layer / File(s)	Summary
Preserve retained funnel series packages/db/src/services/funnel.service.ts, packages/db/src/services/funnel.service.test.ts	toSeries computes the breakdown key before applying the limit. Existing keys continue to receive rows after the limit. Tests verify series counts, all funnel levels, distinct level counts, and the no-breakdown case.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 64b43

Limited funnel breakdowns now retain all levels for accepted series while continuing to cap new series, preventing incorrect flat 100% conversion funnels. The targeted regression coverage supports merge readiness.

Suggested reviewers: lindesvard, niajkitir

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing the funnel breakdown limit from dropping lower funnel steps.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}