Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(experiments): add p-value #28098

Merged
merged 6 commits into from
Feb 4, 2025
Merged

feat(experiments): add p-value #28098

merged 6 commits into from
Feb 4, 2025

Conversation

jurajmajerik
Copy link
Contributor

Changes

Feature flag: experiment-p-value

image image

How did you test this code?

👀

@jurajmajerik jurajmajerik requested review from andehen and a team January 30, 2025 15:21
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added p-value column to experiment results, providing statistical significance information for variant comparisons.

  • Added new pValue column in /frontend/src/scenes/experiments/ExperimentView/SummaryTable.tsx that displays 1 - probability with special formatting for values < 0.001
  • Added EXPERIMENT_P_VALUE feature flag in /frontend/src/lib/constants.tsx to control p-value visibility
  • Column positioned before win probability, maintaining consistent table styling and handling undefined cases
  • P-value calculation helps assess statistical significance of experiment results with 95% confidence level

2 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +283 to +284
const pValue =
result?.probability?.[variantKey] !== undefined ? 1 - result.probability[variantKey] : undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: The p-value calculation (1 - probability) assumes probability represents the complement of the p-value. Verify this assumption with the backend implementation.

Comment on lines +288 to +289
{pValue != undefined ? (
<span className="inline-flex items-center w-52 space-x-4">
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Use strict equality (!==) instead of loose equality (!=) for undefined check to maintain consistency with TypeScript best practices

Suggested change
{pValue != undefined ? (
<span className="inline-flex items-center w-52 space-x-4">
{pValue !== undefined ? (
<span className="inline-flex items-center w-52 space-x-4">

@@ -239,6 +239,7 @@ export const FEATURE_FLAGS = {
LLM_OBSERVABILITY: 'llm-observability', // owner: #team-ai-product-manager
ONBOARDING_SESSION_REPLAY_SEPERATE_STEP: 'onboarding-session-replay-separate-step', // owner: @joshsny #team-growth
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: 'SEPERATE' is misspelled in the feature flag name above - should be 'SEPARATE'

Suggested change
ONBOARDING_SESSION_REPLAY_SEPERATE_STEP: 'onboarding-session-replay-separate-step', // owner: @joshsny #team-growth
ONBOARDING_SESSION_REPLAY_SEPARATE_STEP: 'onboarding-session-replay-separate-step', // owner: @joshsny #team-growth

Copy link
Contributor

github-actions bot commented Jan 30, 2025

Size Change: +40 B (0%)

Total Size: 1.17 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.17 MB +40 B (0%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@jurajmajerik jurajmajerik merged commit 872a07a into master Feb 4, 2025
99 checks passed
@jurajmajerik jurajmajerik deleted the experiment-p-value branch February 4, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants