feat(experiments): support multiple primary metrics #26899
Conversation
…riments-primary-metrics
|
Size Change: 0 B Total Size: 1.11 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated6 snapshot changes in total. 0 added, 6 modified, 0 deleted:
Triggered by this commit. |
…riments-primary-metrics
…riments-primary-metrics
📸 UI snapshots have been updated6 snapshot changes in total. 0 added, 6 modified, 0 deleted:
Triggered by this commit. |
| const cRate = conversionRateForVariant(result, 'control') | ||
| return vRate && cRate ? (vRate - cRate) / cRate : 0 | ||
| }) | ||
| ) |
There was a problem hiding this comment.
Should this math have some tests? The expected behavior isn't super obvious at surface level, so might be good to have some assertions to document intent.
There was a problem hiding this comment.
I've reworked this so the above section has been completely removed.
I've thought more about how the bars should be colored and also checked how our competitors handle this. It turns out that coloring the best bar green doesn’t always make sense, because even the best variant might be entirely in the negative values!
Instead, similar Bayesian tools color the portions of the bar in negative values red and those in positive values green. Here’s how it looks now:
There was a problem hiding this comment.
Here’s how it looks now:
Cool 😄 Worth updating the PR description accordingly?
| @@ -49,6 +50,7 @@ const ResultsTab = (): JSX.Element => { | |||
| )} | |||
| </> | |||
| )} | |||
| <MetricsView /> | |||
There was a problem hiding this comment.
Are you planning to load this behind a feature flag?
There was a problem hiding this comment.
Ah, forgot to push my last commit 🙈
| ) : ( | ||
| <LemonTag size="small" type="danger" className="ml-1"> | ||
| Error | ||
| </LemonTag> |
There was a problem hiding this comment.
I think I'd expect to see this label appear to the left of the text:
Interestingly, no results appear even though they're appearing on my experiment:
There was a problem hiding this comment.
Interesting, what does Error show on hover?
There was a problem hiding this comment.
I think I'd expect to see this label appear to the left of the text
Done!
There was a problem hiding this comment.
Interesting, what does
Errorshow on hover?
I don't see the Error right now but I'm seeing an empty tooltip for "Waiting for experiment to start..."
| : undefined | ||
| } | ||
| > | ||
| Add metric |
There was a problem hiding this comment.
If I add a Trends metric as a second primary metric, how do I specify the exposure event?
There was a problem hiding this comment.
I'll add this in a follow up 👍
…Hog/posthog into experiments-primary-metrics
…riments-primary-metrics
|
@danielbachhuber I've addressed your comments, please have one more look. I've also created an issue to track the todo's before rolling this out: #27014 |
📸 UI snapshots have been updated6 snapshot changes in total. 0 added, 6 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
I feel like these could use a little more padding:
| ) : ( | ||
| <LemonTag size="small" type="danger" className="ml-1"> | ||
| Error | ||
| </LemonTag> |
There was a problem hiding this comment.
Interesting, what does
Errorshow on hover?
I don't see the Error right now but I'm seeing an empty tooltip for "Waiting for experiment to start..."
| // eslint-disable-next-line react/forbid-dom-props | ||
| style={{ fontSize: '10px', fontWeight: 400 }} | ||
| > | ||
| <span>Results loading...</span> |
There was a problem hiding this comment.
| <span>Results loading...</span> | |
| <span>Results loading…</span> |
There was a problem hiding this comment.
I don't see the Error right now but I'm seeing an empty tooltip for "Waiting for experiment to start..."
Good catch, fixed.
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
…Hog/posthog into experiments-primary-metrics
…riments-primary-metrics
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Changes
Support adding/editing/deleting multiple primary metrics + new visualizations.
This is still behind the feature flag and needs more work before rolling out.
Initial state
Primary metrics populated + "no results" diagnostics"
Unexpected error
Dark mode
How did you test this code?
👀