fix(experiments): results page control-wins state, lift colours, dark mode#8091
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughExperiment result derivation now compares treatment probabilities with a conservative control probability and can identify the baseline as the winner. Shared summary logic is used by scorecards and recommendations, while lift colouring depends only on lift sign. Control-win states receive distinct rendering. Hypothesis layout and dark-theme alert styling are also adjusted, with new derivation and summary test coverage. Estimated code review effort: 3 (Moderate) | ~25 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Docker builds report
|
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18694 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #18694 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18694 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18690 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18690 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18690 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18690 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 169bd11e-872a-421e-b5c1-f5171eeee9cb
📒 Files selected for processing (9)
frontend/web/components/experiments/results/ExperimentDetailHeader.tsxfrontend/web/components/experiments/results/ExperimentMetricScorecard.tsxfrontend/web/components/experiments/results/ExperimentRecommendation.tsxfrontend/web/components/experiments/results/ExperimentResultsAxisChart.tsxfrontend/web/components/experiments/results/ExperimentResultsScorecardTable.tsxfrontend/web/components/experiments/results/ExperimentSummaryScorecard.tsxfrontend/web/components/experiments/results/__tests__/derive.test.tsfrontend/web/components/experiments/results/derive.tsfrontend/web/styles/project/_alert.scss
💤 Files with no reviewable changes (1)
- frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx
|
@themis-blindfold review |
⚖️ Themis judgement: ✅ Ship itClean feature PR that adds a control-wins state to the experiment results page, simplifies lift colouring to sign-only, and fixes dark mode readability for the success alert banner. The core logic in One gap I independently verified: the new
📝 Walkthrough
🧪 How to verify
Automate: The new derive tests cover the logic; a Chromatic or visual regression snapshot of the warning banner in dark mode would lock in the contrast fix once the Product take: Solid improvement — experiments that confirm the control is winning now get a clear signal instead of a confusing absence of recommendation. Simplifying lift colours to sign-only is the right call while 🧭 Assumptions & unverified claims
Control wins this round — and now the UI knows it too. · reviewed at 5e8066b |
… text in dark mode
|
Reviewers comparison
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/web/components/experiments/results/derive.ts (1)
157-176: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not treat missing treatment inference as control probability.
Lines 160-162 skip treatments without inference, then Lines 175-176 assign the remaining probability mass to Control. Because missing inference is a supported state, this can falsely select Control when a treatment is still collecting data. Return an insufficient-data result unless every treatment has inference before calculating the residual control probability.
Proposed fix
let best: WinningVariant | null = null let treatmentChancesTotal = 0 + if ( + identities.some( + (variant) => !variant.isControl && !metricResult.inference[variant.key], + ) + ) { + return null + } for (const v of identities) { if (v.isControl) continue const inf = metricResult.inference[v.key] - if (!inf) continue treatmentChancesTotal += inf.chance_to_win
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a150032f-1e68-4bc0-8731-412b0d98578a
📒 Files selected for processing (4)
frontend/web/components/experiments/results/ExperimentResultsAxisChart.tsxfrontend/web/components/experiments/results/ExperimentResultsScorecardTable.tsxfrontend/web/components/experiments/results/derive.tsfrontend/web/styles/project/_alert.scss
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
UI fixes for the experiment results page:
expected_directionno longer affects colouring..alert-success stronghad no dark override).How did you test this code?
Unit tests for the winner/summary derivation; manually checked the control-wins state, negative lift colours, and dark mode.
Before

After
