Skip to content

feat(experiments): add metric breakdown injector for mean metrics - #62291

Draft
rodrigoi wants to merge 2 commits into
experiments/breakdown-attribution-metric-breakdown-injector-for-funnelsfrom
experiments/breakdown-attribution-mean-metric-breakdown-injector
Draft

rodrigoi wants to merge 2 commits into
experiments/breakdown-attribution-metric-breakdown-injector-for-funnelsfrom
experiments/breakdown-attribution-mean-metric-breakdown-injector

Conversation

@rodrigoi

@rodrigoi rodrigoi commented Jun 9, 2026 •

Copy link
Copy Markdown
Member

Problem

#61796 introduced metric-event breakdown attribution for funnel metrics. Mean metrics still attribute the breakdown from the exposure event, with the same null-bucket and consistency problems.

Changes

This PR migrates mean metrics to MetricBreakdownInjector, still behind the experiment-metric-event-breakdowns flag.

Mean support in the injector

Mean metrics have no attribution modes, so the breakdown is always first touch: argMin over the metric event timestamp per user. The conversion window is already enforced by the entity_metrics join, so no extra condition is needed. The injector's constructor now accepts all metric types; the funnel-only _attribution_step asserts it only runs for funnels.

Session-property metrics

Session-property means have an extra metric_events_by_session layer whose metric_events CTE joins exposures and can't see raw event properties. The breakdown is read and deduped per session in the by-session layer (via any()), then carried up with first_event_timestamp so attribution uses the session's first event time.

Winsorization

Winsorized means carry the breakdown through the percentiles and winsorized_entity_metrics CTEs, preserving per-breakdown-group thresholds. Top-N ranking is always computed from entity_metrics since winsorization doesn't change the set of (entity, variant, breakdown) tuples; _inject_final_breakdown_columns learned a final_cte_name so the outer SELECT reads from the right CTE.

Wiring

Mean joins the migrated set in the runner. Both injectors expose inject_mean_breakdown_columns, so the mean builder calls whichever it was given without type checks.

  • products/experiments/backend/hogql_queries/metric_breakdown_injector.py
  • products/experiments/backend/hogql_queries/experiment_query_runner.py
  • products/experiments/backend/hogql_queries/experiment_mean_query_builder.py

Tests

test_metric_breakdown.py starts the flag-on runner suite (funnel and mean paths, including session properties and winsorization); injector unit tests cover the new AST shapes. The flag-off suite in test_breakdown.py stays as the old-path regression guard.

  • products/experiments/backend/hogql_queries/test/experiment_query_runner/test_metric_breakdown.py
  • products/experiments/backend/hogql_queries/test/experiment_query_runner/test_breakdown.py
  • products/experiments/backend/hogql_queries/test/test_metric_breakdown_injector.py

How did you test this code?

pnpm turbo run backend:test --filter=@posthog/products-experiments

cat-type-small

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Model: Opus 4.7 (implementation), Fable 5 (restack and PR description)
Manually refactored: yes

Skills used:

  • /writing-plans (superpowers)
  • /writing-tests (local)
  • /writing-pull-requests (local)

Relevant decisions:

  • Mean attribution is fixed first touch from the metric event, with no user-facing modes; funnels are the only type where an attribution choice is meaningful.
  • Winsorization thresholds are computed per breakdown group (unchanged from the old injector), and top-N ranking deliberately reads pre-winsorization entity_metrics.
  • Session-property breakdowns are deduped with any() per session, mirroring how session_value is computed.

rodrigoi commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

@greptile-apps

greptile-apps Bot commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor

Comments Outside Diff (1)

  1. products/experiments/backend/hogql_queries/metric_breakdown_injector.py, line 1-13 (link)

    P2 The module docstring still says "Scope: funnel metrics only" but mean metrics were just migrated here, and MetricType already includes ExperimentRatioMetric and ExperimentRetentionMetric in the union for future migrations. The scope line and first-paragraph framing no longer reflect the file's actual responsibility.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: products/experiments/backend/hogql_queries/metric_breakdown_injector.py
    Line: 1-13
    
    Comment:
    The module docstring still says "Scope: funnel metrics only" but mean metrics were just migrated here, and `MetricType` already includes `ExperimentRatioMetric` and `ExperimentRetentionMetric` in the union for future migrations. The scope line and first-paragraph framing no longer reflect the file's actual responsibility.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
products/experiments/backend/hogql_queries/metric_breakdown_injector.py:1-13
The module docstring still says "Scope: funnel metrics only" but mean metrics were just migrated here, and `MetricType` already includes `ExperimentRatioMetric` and `ExperimentRetentionMetric` in the union for future migrations. The scope line and first-paragraph framing no longer reflect the file's actual responsibility.

Reviews (1): Last reviewed commit: "feat(experiments): add metric breakdown ..." | Re-trigger Greptile

@scheduled-actions-posthog

This comment was marked as outdated.

@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from 23e0d21 to 48d1526 Compare June 16, 2026 22:54
@scheduled-actions-posthog

This comment was marked as outdated.

@scheduled-actions-posthog

This comment was marked as outdated.

@rodrigoi rodrigoi added waiting Prevents stale-bot from marking the PR as stale. and removed stale labels Jul 3, 2026
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from 48d1526 to fbd4113 Compare July 16, 2026 01:45
@trunk-io

trunk-io Bot commented Jul 16, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@rodrigoi rodrigoi changed the title feat(experiments): add metric breakdown injector for mean metrics. feat(experiments): add metric breakdown injector for mean metrics Jul 16, 2026
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from fbd4113 to 7fc7e13 Compare July 16, 2026 16:19
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from 7fc7e13 to 0fa75b9 Compare July 16, 2026 20:27
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from 0fa75b9 to 68e8ee2 Compare July 16, 2026 20:43
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from 8fa48db to 1d465a7 Compare July 18, 2026 16:53
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch 2 times, most recently from ab9ccd1 to f24fbc3 Compare July 20, 2026 01:54
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from f24fbc3 to 6cfc044 Compare July 21, 2026 05:22
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch 2 times, most recently from a309988 to 676ed8b Compare July 23, 2026 14:43

@andehen andehen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings from a review of the full stack (#61788, #61796, #62291, #62292, #62293), placed on the PR where each belongs. Prefixes: should-fix, suggestion, nit.

@scheduled-actions-posthog

Copy link
Copy Markdown
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

@scheduled-actions-posthog

Copy link
Copy Markdown
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from 676ed8b to d5532db Compare August 14, 2026 21:21
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-mean-metric-breakdown-injector branch from d5532db to 57d2d2b Compare August 15, 2026 04:21
@scheduled-actions-posthog

Copy link
Copy Markdown
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

@rodrigoi rodrigoi added waiting Prevents stale-bot from marking the PR as stale. and removed stale labels Aug 25, 2026
@rodrigoi
rodrigoi marked this pull request as draft October 29, 2026 17:48

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting Prevents stale-bot from marking the PR as stale.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants