Skip to content

feat(experiments): add retention metrics to the metric breakdown injector - #62292

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

rodrigoi wants to merge 2 commits into
experiments/breakdown-attribution-mean-metric-breakdown-injectorfrom
experiments/breakdown-attribution-retention-metric-breakdown-injector

Conversation

@rodrigoi

@rodrigoi rodrigoi commented Jun 9, 2026 •

Copy link
Copy Markdown
Member

Problem

Funnel (#61796) and mean (#62291) metrics attribute breakdowns from the metric event. Retention metrics still use the exposure-event injector.

Changes

This PR migrates retention metrics to MetricBreakdownInjector, behind the same flag.

Retention support in the injector

The breakdown is read off the start event and attributed first touch (argMin over the start event timestamp). Attribution is fixed with no user choice, and that's deliberate: retention is retained / cohort, the cohort (denominator) is defined by the start event, and every retained user has one. Reading the breakdown off the completion event would break the per-bucket denominator, since non-retained users have no completion event to bucket by. start_events already groups by entity, so the value is deduped per user there and carried into entity_metrics.

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

Tests

Flag-on retention cases in test_metric_breakdown.py (start-event attribution, limit, Other bucket) plus injector unit tests.

  • products/experiments/backend/hogql_queries/test/experiment_query_runner/test_metric_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-tests (local)
  • /writing-pull-requests (local)

Relevant decisions:

  • Start-event attribution is the only source that keeps the per-bucket denominator well-defined, so no attribution dropdown is offered for retention.

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 Stale module-level docstring

    The module header still says "Scope: funnel metrics only" and "Metric-event breakdown injection for experiment funnel queries," but the class has been extended to handle mean, ratio, and now retention metrics. Any new reader will come away with an inaccurate mental model of the file's scope.

    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:
    **Stale module-level docstring**
    
    The module header still says "Scope: funnel metrics only" and "Metric-event breakdown injection for experiment funnel queries," but the class has been extended to handle mean, ratio, and now retention metrics. Any new reader will come away with an inaccurate mental model of the file's scope.
    
    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
**Stale module-level docstring**

The module header still says "Scope: funnel metrics only" and "Metric-event breakdown injection for experiment funnel queries," but the class has been extended to handle mean, ratio, and now retention metrics. Any new reader will come away with an inaccurate mental model of the file's scope.

Reviews (1): Last reviewed commit: "feat(experiments): add ratio metrics to ..." | Re-trigger Greptile

@scheduled-actions-posthog

This comment was marked as outdated.

@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from b2daac5 to 819b8db 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-retention-metric-breakdown-injector branch from 819b8db to cfb273c 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 ratio metrics to the metric breakdown injector. feat(experiments): add retention metrics to the metric breakdown injector Jul 16, 2026
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from cfb273c to e0c91db Compare July 16, 2026 16:19
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from e0c91db to 2df015d Compare July 16, 2026 20:27
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from 2df015d to ebfae47 Compare July 16, 2026 20:43
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from 29d5be1 to 3eb64ae Compare July 18, 2026 16:53
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from 3eb64ae to e09f392 Compare July 18, 2026 22:01
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from e09f392 to 20f5ab8 Compare July 20, 2026 01:54
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from 20f5ab8 to 3d9888b Compare July 21, 2026 05:22
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from 3d9888b to 095a579 Compare July 21, 2026 05:30
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from 095a579 to ced8686 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. Only one small thing here, the retention path is the most solid of the four (breakdown read off the start event keeps the per-bucket denominator well defined, and the non-retained-users test covers it).

@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-retention-metric-breakdown-injector branch from ced8686 to 59c3368 Compare August 14, 2026 21:21
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-retention-metric-breakdown-injector branch from 59c3368 to ef22067 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:49

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