From 36d16e636d7517dcfc95d879716f53a3cf07f759 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 24 Jul 2026 15:25:52 +0200 Subject: [PATCH 1/3] feat: colour experiment lift by metric direction --- frontend/common/types/responses.ts | 1 + .../results/ExperimentMetricScorecard.tsx | 1 + .../results/ExperimentResultsAxisChart.tsx | 6 ++- .../ExperimentResultsScorecardTable.tsx | 6 ++- .../results/ExperimentSummaryScorecard.tsx | 5 +-- .../results/__tests__/derive.test.ts | 22 +++++++++- .../components/experiments/results/derive.ts | 43 ++++++++++++++----- 7 files changed, 64 insertions(+), 20 deletions(-) diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 76a6d722ebb9..624d96d77b06 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -671,6 +671,7 @@ export type ExperimentMetric = { metric: number metric_name: string aggregation: MetricAggregation + direction: MetricDirection expected_direction: ExpectedDirection created_at: string } diff --git a/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx b/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx index 2499dc46f20a..53485744eacc 100644 --- a/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx +++ b/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx @@ -55,6 +55,7 @@ const ExperimentMetricScorecard: FC = ({ <> {metricResult && ( = ({ + direction, identities, metricName, metricResult, @@ -87,7 +89,7 @@ const ExperimentResultsAxisChart: FC = ({ ) } if (!inf) return null - const colour = getLiftColour(inf.lift) + const colour = getLiftColour(inf.lift, direction) const ciLeft = valueToPercent(inf.ci_low, range) const ciRight = valueToPercent(inf.ci_high, range) const dotPos = valueToPercent(inf.lift, range) diff --git a/frontend/web/components/experiments/results/ExperimentResultsScorecardTable.tsx b/frontend/web/components/experiments/results/ExperimentResultsScorecardTable.tsx index 3c3935f4abb1..882fcab4b287 100644 --- a/frontend/web/components/experiments/results/ExperimentResultsScorecardTable.tsx +++ b/frontend/web/components/experiments/results/ExperimentResultsScorecardTable.tsx @@ -12,6 +12,7 @@ import { ExperimentMetric, Inference, MetricAggregation, + MetricDirection, VariantStats, } from 'common/types/responses' import { @@ -43,6 +44,7 @@ const renderMetricValue = ( const renderLift = ( identity: VariantIdentity, inference: Inference | null, + direction: MetricDirection, liftRange: number, ): ReactNode => { if (identity.isControl) { @@ -51,7 +53,7 @@ const renderLift = ( if (!inference) { return Collecting data… } - const colour = getLiftColour(inference.lift) + const colour = getLiftColour(inference.lift, direction) const left = liftToPercent(inference.ci_low, liftRange) const right = liftToPercent(inference.ci_high, liftRange) const dotPos = liftToPercent(inference.lift, liftRange) @@ -195,7 +197,7 @@ const ExperimentResultsScorecardTable: FC< {stats ? stats.n.toLocaleString() : '—'} {renderMetricValue(stats, metric.aggregation)} - {renderLift(v, inference, liftRange)} + {renderLift(v, inference, metric.direction, liftRange)} {renderCI(v, inference)} {renderWinProbability(v, inference, v.key === winnerKey)} diff --git a/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx b/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx index b374357c7e78..3b8879d9cbaa 100644 --- a/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx +++ b/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx @@ -22,9 +22,8 @@ const ExperimentSummaryScorecard: FC = ({ const hasResults = !!results let liftClassName: string | undefined - if (summary && !summary.controlWins) { - liftClassName = summary.liftFavourable ? 'text-success' : 'text-danger' - } + if (summary?.liftTone === 'success') liftClassName = 'text-success' + if (summary?.liftTone === 'danger') liftClassName = 'text-danger' return ( <> diff --git a/frontend/web/components/experiments/results/__tests__/derive.test.ts b/frontend/web/components/experiments/results/__tests__/derive.test.ts index 99cf3627e07d..e5669fe2e74a 100644 --- a/frontend/web/components/experiments/results/__tests__/derive.test.ts +++ b/frontend/web/components/experiments/results/__tests__/derive.test.ts @@ -4,6 +4,7 @@ import { computeLiftRange, computeAxisRange, buildExposuresChartData, + LiftTone, deriveSummary, formatBucketLabel, getHeadlineTotal, @@ -19,6 +20,7 @@ import { Experiment, ExperimentFeature, ExposuresSummary, + MetricDirection, MultivariateOption, } from 'common/types/responses' @@ -300,6 +302,7 @@ describe('deriveSummary', () => { { aggregation: 'occurrence', created_at: '2026-06-01T00:00:00Z', + direction: 'up', expected_direction: 'increase', id: 1, metric: 1, @@ -320,7 +323,7 @@ describe('deriveSummary', () => { expect(deriveSummary(experiment, results(metricResult))).toMatchObject({ chanceToBest: '75%', controlWins: false, - liftFavourable: true, + liftTone: 'success', liftVsControl: '+8.0%', winnerName: 'b', }) @@ -332,11 +335,26 @@ describe('deriveSummary', () => { ).toMatchObject({ chanceToBest: '85%', controlWins: true, - liftFavourable: false, + liftTone: 'neutral', liftVsControl: 'Baseline', winnerName: 'Control', }) }) + + it.each<[MetricDirection, LiftTone]>([ + ['up', 'success'], + ['down', 'danger'], + ['informational', 'neutral'], + ])( + 'tones the positive winning lift for a %s metric as %s', + (direction, tone) => { + const exp: Experiment = { + ...experiment, + metrics: [{ ...experiment.metrics[0], direction }], + } + expect(deriveSummary(exp, results(metricResult))?.liftTone).toBe(tone) + }, + ) }) describe('computeLiftRange', () => { diff --git a/frontend/web/components/experiments/results/derive.ts b/frontend/web/components/experiments/results/derive.ts index 80bd9552dd1b..f5e335834a3c 100644 --- a/frontend/web/components/experiments/results/derive.ts +++ b/frontend/web/components/experiments/results/derive.ts @@ -1,6 +1,10 @@ import moment from 'moment' import { ChartDataPoint, buildChartColorMap } from 'components/charts' -import { colorTextDanger, colorTextSuccess } from 'common/theme/tokens' +import { + colorTextDanger, + colorTextSecondary, + colorTextSuccess, +} from 'common/theme/tokens' import { BayesianMetricResult, BayesianResultsSummary, @@ -9,6 +13,7 @@ import { ExposureGranularity, ExposuresSummary, Inference, + MetricDirection, MultivariateOption, } from 'common/types/responses' import { getPrimaryMetric } from 'components/experiments/constants' @@ -120,12 +125,21 @@ export const getResultsTotalUsers = ( return Object.values(firstMetric.variants).reduce((sum, v) => sum + v.n, 0) } -// Colour by sign only — expected_direction is not used reliably yet, so it -// deliberately plays no part in lift colouring. -export const isLiftFavourable = (lift: number): boolean => lift > 0 - -export const getLiftColour = (lift: number): string => - isLiftFavourable(lift) ? colorTextSuccess : colorTextDanger +// A lift is favourable when it moves with the metric's inherent polarity +// (direction), e.g. a drop in a lower-is-better metric. The experiment-level +// expected_direction guardrail deliberately plays no part in colouring. +export const isLiftFavourable = ( + lift: number, + direction: MetricDirection, +): boolean => (direction === 'down' ? lift < 0 : lift > 0) + +export const getLiftColour = ( + lift: number, + direction: MetricDirection, +): string => { + if (direction === 'informational') return colorTextSecondary + return isLiftFavourable(lift, direction) ? colorTextSuccess : colorTextDanger +} export const formatLiftPct = (lift: number): string => { const pct = lift * 100 @@ -185,6 +199,8 @@ export const getWinningVariant = ( return best } +export type LiftTone = 'success' | 'danger' | 'neutral' + export type SummaryStats = { winnerName: string winnerColour: string @@ -192,7 +208,7 @@ export type SummaryStats = { controlWins: boolean chanceToBest: string liftVsControl: string - liftFavourable: boolean + liftTone: LiftTone } export const deriveSummary = ( @@ -211,13 +227,18 @@ export const deriveSummary = ( const winnerIdentity = identities.find((v) => v.key === winner.key) const controlIdentity = identities.find((v) => v.isControl) + let liftTone: LiftTone = 'neutral' + if (winner.inference && metric.direction !== 'informational') { + liftTone = isLiftFavourable(winner.inference.lift, metric.direction) + ? 'success' + : 'danger' + } + return { chanceToBest: `${Math.round(winner.chanceToWin * 100)}%`, controlColour: controlIdentity?.colour ?? '', controlWins: winner.isControl, - liftFavourable: winner.inference - ? isLiftFavourable(winner.inference.lift) - : false, + liftTone, liftVsControl: winner.inference ? formatLiftPct(winner.inference.lift) : 'Baseline', From 20db5e0767750e88043dfdf4b0e84b5aa455e946 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 24 Jul 2026 16:28:27 +0200 Subject: [PATCH 2/3] fix: make experiment metric direction optional with up fallback --- frontend/common/types/responses.ts | 3 ++- .../results/ExperimentMetricScorecard.tsx | 2 +- .../ExperimentResultsScorecardTable.tsx | 9 ++++++++- .../results/__tests__/derive.test.ts | 18 +++++++++++++++++- .../components/experiments/results/derive.ts | 5 +++-- 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 624d96d77b06..feef2b97213d 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -671,7 +671,8 @@ export type ExperimentMetric = { metric: number metric_name: string aggregation: MetricAggregation - direction: MetricDirection + // Absent from API responses until the backend exposes it; treat as 'up'. + direction?: MetricDirection expected_direction: ExpectedDirection created_at: string } diff --git a/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx b/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx index 53485744eacc..b60f4d8024c3 100644 --- a/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx +++ b/frontend/web/components/experiments/results/ExperimentMetricScorecard.tsx @@ -55,7 +55,7 @@ const ExperimentMetricScorecard: FC = ({ <> {metricResult && ( {stats ? stats.n.toLocaleString() : '—'} {renderMetricValue(stats, metric.aggregation)} - {renderLift(v, inference, metric.direction, liftRange)} + + {renderLift( + v, + inference, + metric.direction ?? 'up', + liftRange, + )} + {renderCI(v, inference)} {renderWinProbability(v, inference, v.key === winnerKey)} diff --git a/frontend/web/components/experiments/results/__tests__/derive.test.ts b/frontend/web/components/experiments/results/__tests__/derive.test.ts index e5669fe2e74a..29cb2d67d7c8 100644 --- a/frontend/web/components/experiments/results/__tests__/derive.test.ts +++ b/frontend/web/components/experiments/results/__tests__/derive.test.ts @@ -11,6 +11,7 @@ import { getVariantIdentities, getVariantTotals, getWinningVariant, + isLiftFavourable, liftToPercent, valueToPercent, } from 'components/experiments/results/derive' @@ -341,10 +342,11 @@ describe('deriveSummary', () => { }) }) - it.each<[MetricDirection, LiftTone]>([ + it.each<[MetricDirection | undefined, LiftTone]>([ ['up', 'success'], ['down', 'danger'], ['informational', 'neutral'], + [undefined, 'success'], // legacy payloads without direction default to up ])( 'tones the positive winning lift for a %s metric as %s', (direction, tone) => { @@ -357,6 +359,20 @@ describe('deriveSummary', () => { ) }) +describe('isLiftFavourable', () => { + it.each<[number, MetricDirection, boolean]>([ + [0.08, 'up', true], + [-0.08, 'up', false], + [-0.08, 'down', true], + [0.08, 'down', false], + ])( + 'judges a %d lift on a %s metric as favourable=%s', + (lift, direction, expected) => { + expect(isLiftFavourable(lift, direction)).toBe(expected) + }, + ) +}) + describe('computeLiftRange', () => { it('pads the largest treatment interval magnitude beyond the default range', () => { expect( diff --git a/frontend/web/components/experiments/results/derive.ts b/frontend/web/components/experiments/results/derive.ts index f5e335834a3c..a9664a78d09c 100644 --- a/frontend/web/components/experiments/results/derive.ts +++ b/frontend/web/components/experiments/results/derive.ts @@ -227,9 +227,10 @@ export const deriveSummary = ( const winnerIdentity = identities.find((v) => v.key === winner.key) const controlIdentity = identities.find((v) => v.isControl) + const direction = metric.direction ?? 'up' let liftTone: LiftTone = 'neutral' - if (winner.inference && metric.direction !== 'informational') { - liftTone = isLiftFavourable(winner.inference.lift, metric.direction) + if (winner.inference && direction !== 'informational') { + liftTone = isLiftFavourable(winner.inference.lift, direction) ? 'success' : 'danger' } From 2b2c5b39faf3aef29682c9d3665de511c61844cf Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 27 Jul 2026 16:17:56 +0200 Subject: [PATCH 3/3] fix: default lift class to neutral text-secondary --- .../experiments/results/ExperimentSummaryScorecard.tsx | 2 +- .../components/experiments/results/__tests__/derive.test.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx b/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx index 3b8879d9cbaa..1c42ac8f044a 100644 --- a/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx +++ b/frontend/web/components/experiments/results/ExperimentSummaryScorecard.tsx @@ -21,7 +21,7 @@ const ExperimentSummaryScorecard: FC = ({ ) const hasResults = !!results - let liftClassName: string | undefined + let liftClassName = 'text-secondary' // neutral if (summary?.liftTone === 'success') liftClassName = 'text-success' if (summary?.liftTone === 'danger') liftClassName = 'text-danger' diff --git a/frontend/web/components/experiments/results/__tests__/derive.test.ts b/frontend/web/components/experiments/results/__tests__/derive.test.ts index 29cb2d67d7c8..8e37fc86d67e 100644 --- a/frontend/web/components/experiments/results/__tests__/derive.test.ts +++ b/frontend/web/components/experiments/results/__tests__/derive.test.ts @@ -25,6 +25,8 @@ import { MultivariateOption, } from 'common/types/responses' +type OptionalMetricDirection = MetricDirection | undefined + const option = (over: Partial): MultivariateOption => ({ boolean_value: undefined, default_percentage_allocation: 0, @@ -342,7 +344,7 @@ describe('deriveSummary', () => { }) }) - it.each<[MetricDirection | undefined, LiftTone]>([ + it.each<[OptionalMetricDirection, LiftTone]>([ ['up', 'success'], ['down', 'danger'], ['informational', 'neutral'],