Skip to content

feat(experiments): add ratio metrics to the metric breakdown injector - #62293

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

rodrigoi wants to merge 3 commits into
experiments/breakdown-attribution-retention-metric-breakdown-injectorfrom
experiments/breakdown-attribution-ratio-metric-breakdown-injector

Conversation

@rodrigoi

@rodrigoi rodrigoi commented Jun 9, 2026 •

Copy link
Copy Markdown
Member

Problem

Ratio metrics are the last type still attributing breakdowns from the exposure event; funnels, mean, and retention migrated in #61796, #62291, #62292.

Changes

This PR migrates ratio metrics to MetricBreakdownInjector, completing the per-type migration behind the experiment-metric-event-breakdowns flag.

Ratio support in the injector

Ratio is Σnumerator / Σdenominator over two event streams, pre-aggregated per entity before joining exposures. To keep one experiment unit in exactly one bucket (so per-breakdown rows sum to the overall result), the whole user is attributed by their numerator event: read off numerator_events, first touch via argMin in numerator_agg, carried through entity_metrics with any(). Winsorized ratio queries reuse the mean winsorization carry-through helper.

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

Migration complete

All four metric types now use the metric-event injector when the flag is on. Deleting the old BreakdownInjector is a follow-up once the flag rolls out.

Tests

Flag-on ratio cases in test_metric_breakdown.py (numerator attribution, winsorization, 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:

  • Numerator-event attribution keeps each experiment unit in a single breakdown bucket, which is what makes per-breakdown results sum to the overall ratio.
  • The denominator is never a breakdown source; a user can have denominator events in several buckets, which would double-count units.

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/test/experiment_query_runner/test_metric_breakdown.py, line 607-612 (link)

    P2 Denominator-only bucket not asserted

    The test verifies Chrome is present but doesn't assert what bucket denominator-only users land in. Because these users have no numerator event, any(null) in entity_metrics yields null for their breakdown column. The final if(in_top, value, other) expression evaluates null IN (subquery) as null, which routes them to the BREAKDOWN_OTHER_STRING_LABEL bucket — they are not dropped, and their denominator_value is included in "Other". Adding an assertion like self.assertIn([BREAKDOWN_OTHER_STRING_LABEL], breakdown_values) would lock in this behavior and prevent a silent regression if the attribution logic changes.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: products/experiments/backend/hogql_queries/test/experiment_query_runner/test_metric_breakdown.py
    Line: 607-612
    
    Comment:
    **Denominator-only bucket not asserted**
    
    The test verifies Chrome is present but doesn't assert what bucket denominator-only users land in. Because these users have no numerator event, `any(null)` in `entity_metrics` yields null for their breakdown column. The final `if(in_top, value, other)` expression evaluates `null IN (subquery)` as null, which routes them to the `BREAKDOWN_OTHER_STRING_LABEL` bucket — they are *not* dropped, and their `denominator_value` is included in "Other". Adding an assertion like `self.assertIn([BREAKDOWN_OTHER_STRING_LABEL], breakdown_values)` would lock in this behavior and prevent a silent regression if the attribution logic changes.
    
    How can I resolve this? If you propose a fix, please make it concise.
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/test/experiment_query_runner/test_metric_breakdown.py:607-612
**Denominator-only bucket not asserted**

The test verifies Chrome is present but doesn't assert what bucket denominator-only users land in. Because these users have no numerator event, `any(null)` in `entity_metrics` yields null for their breakdown column. The final `if(in_top, value, other)` expression evaluates `null IN (subquery)` as null, which routes them to the `BREAKDOWN_OTHER_STRING_LABEL` bucket — they are *not* dropped, and their `denominator_value` is included in "Other". Adding an assertion like `self.assertIn([BREAKDOWN_OTHER_STRING_LABEL], breakdown_values)` would lock in this behavior and prevent a silent regression if the attribution logic changes.

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

@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-ratio-metric-breakdown-injector branch from b48e6f4 to d86aa5b 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-ratio-metric-breakdown-injector branch from d86aa5b to 6da73b0 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 ratio metrics to the metric breakdown injector Jul 16, 2026
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from 6da73b0 to bebbd53 Compare July 16, 2026 16:19
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from bebbd53 to 1470373 Compare July 16, 2026 20:27
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch 2 times, most recently from 6800589 to 9c8de5d Compare July 16, 2026 20:48
@rodrigoi
rodrigoi requested a review from a team July 17, 2026 05:00
@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Backend coverage — 98.0% of changed backend lines covered — 25 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 98.0% (1,316 / 1,341)

File Patch Uncovered changed lines
products/experiments/backend/hogql_queries/metric_breakdown_injector.py 88.7% 60, 74, 239–240, 242–243, 245–249, 251–255, 259, 264, 308, 355, 431, 450, 481, 500
products/experiments/backend/hogql_queries/test/test_metric_breakdown_injector.py 99.7% 351

🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 31864211158 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)
Product Coverage Lines
platform_features ██░░░░░░░░░░░░░░░░░░ 12.1% 7 / 58
warehouse_sources_queue ███████░░░░░░░░░░░░░ 36.1% 91 / 252
batch_exports ████████░░░░░░░░░░░░ 39.2% 8,862 / 22,581
demo ███████████░░░░░░░░░ 56.3% 1,497 / 2,661
data_tools █████████████░░░░░░░ 66.2% 90 / 136
tasks ██████████████░░░░░░ 70.2% 38,633 / 55,017
ai_gateway ███████████████░░░░░ 75.0% 9 / 12
signals █████████████████░░░ 83.1% 28,098 / 33,794
apm █████████████████░░░ 83.9% 1,222 / 1,457
cdp █████████████████░░░ 84.2% 3,914 / 4,649
actions █████████████████░░░ 86.6% 717 / 828
wizard █████████████████░░░ 86.7% 1,090 / 1,257
data_modeling █████████████████░░░ 86.9% 9,631 / 11,082
notebooks █████████████████░░░ 87.2% 8,547 / 9,806
cohorts ██████████████████░░ 87.6% 6,528 / 7,452
product_tours ██████████████████░░ 87.9% 1,303 / 1,482
managed_warehouse ██████████████████░░ 88.8% 6,659 / 7,503
data_warehouse ██████████████████░░ 88.8% 12,468 / 14,043
business_knowledge ██████████████████░░ 89.0% 4,384 / 4,928
exports ██████████████████░░ 89.2% 7,597 / 8,520
engineering_analytics ██████████████████░░ 89.4% 6,839 / 7,649
dashboards ██████████████████░░ 89.5% 6,221 / 6,954
visual_review ██████████████████░░ 89.5% 5,870 / 6,558
mcp_analytics ██████████████████░░ 90.4% 3,968 / 4,388
alerts ██████████████████░░ 90.6% 4,676 / 5,163
links ██████████████████░░ 90.6% 183 / 202
streamlit_apps ██████████████████░░ 90.7% 2,630 / 2,901
conversations ██████████████████░░ 90.8% 19,958 / 21,989
error_tracking ██████████████████░░ 91.2% 11,345 / 12,435
stamphog ██████████████████░░ 91.3% 4,505 / 4,936
slack_app ██████████████████░░ 92.4% 11,278 / 12,212
managed_migrations ███████████████████░ 92.6% 1,556 / 1,681
early_access_features ███████████████████░ 92.9% 1,347 / 1,450
notifications ███████████████████░ 92.9% 1,060 / 1,141
web_analytics ███████████████████░ 92.9% 16,337 / 17,578
ai_observability ███████████████████░ 93.1% 17,445 / 18,744
mcp_store ███████████████████░ 93.1% 7,411 / 7,957
canvas ███████████████████░ 93.2% 3,328 / 3,572
posthog_ai ███████████████████░ 93.2% 1,326 / 1,422
surveys ███████████████████░ 93.3% 5,943 / 6,373
reminders ███████████████████░ 93.4% 468 / 501
marketing_analytics ███████████████████░ 93.5% 16,413 / 17,559
approvals ███████████████████░ 93.5% 3,491 / 3,734
product_analytics ███████████████████░ 93.6% 7,126 / 7,614
review_hog ███████████████████░ 94.1% 9,383 / 9,973
legal_documents ███████████████████░ 94.1% 1,926 / 2,047
endpoints ███████████████████░ 94.3% 8,814 / 9,349
workflows ███████████████████░ 94.4% 8,084 / 8,564
tracing ███████████████████░ 94.4% 2,678 / 2,837
skills ███████████████████░ 94.7% 3,869 / 4,085
annotations ███████████████████░ 95.1% 732 / 770
data_quality ███████████████████░ 95.3% 1,457 / 1,529
logs ███████████████████░ 95.5% 12,644 / 13,244
messaging ███████████████████░ 95.6% 3,423 / 3,580
experiments ███████████████████░ 95.6% 30,226 / 31,608
replay_vision ███████████████████░ 95.9% 21,571 / 22,492
customer_analytics ███████████████████░ 96.1% 13,646 / 14,199
revenue_analytics ███████████████████░ 96.3% 1,887 / 1,960
user_interviews ███████████████████░ 96.5% 2,638 / 2,734
feature_flags ███████████████████░ 96.6% 18,993 / 19,653
access_control ███████████████████░ 96.7% 873 / 903
growth ███████████████████░ 96.7% 6,494 / 6,717
warehouse_sources ████████████████████ 97.5% 393,119 / 403,174
data_catalog ████████████████████ 97.8% 2,747 / 2,808
analytics_platform ████████████████████ 98.1% 2,254 / 2,298
metrics ████████████████████ 98.2% 2,491 / 2,536
pulse ████████████████████ 98.4% 2,017 / 2,049
live_debugger ████████████████████ 99.2% 613 / 618
field_notes ████████████████████ 99.4% 158 / 159

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from ce4ccb3 to cdf7536 Compare July 17, 2026 18:34
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from cdf7536 to 022b2ef Compare July 18, 2026 00:34
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from 022b2ef to 4452783 Compare July 18, 2026 06:51
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from 4452783 to 8d31818 Compare July 18, 2026 16:53
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch 2 times, most recently from 82a0c72 to ea30baa Compare July 20, 2026 01:54
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from ea30baa to f00ddd1 Compare July 21, 2026 05:22
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from f00ddd1 to 4056187 Compare July 21, 2026 05:30
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from 4056187 to 5befb66 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.

@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from 5befb66 to 149a2ae Compare August 14, 2026 21:21
@rodrigoi
rodrigoi force-pushed the experiments/breakdown-attribution-ratio-metric-breakdown-injector branch from 149a2ae to 904f970 Compare August 15, 2026 04:21
@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