Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions frontend/common/types/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,8 @@ export type ExperimentMetric = {
metric: number
metric_name: string
aggregation: MetricAggregation
// Absent from API responses until the backend exposes it; treat as 'up'.
direction?: MetricDirection
expected_direction: ExpectedDirection
created_at: string
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const ExperimentMetricScorecard: FC<ExperimentMetricScorecardProps> = ({
<>
{metricResult && (
<ExperimentResultsAxisChart
direction={metric.direction ?? 'up'}
identities={identities}
metricName={metric.metric_name}
metricResult={metricResult}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FC, useMemo } from 'react'
import ColorSwatch from 'components/ColorSwatch'
import { BayesianMetricResult } from 'common/types/responses'
import { BayesianMetricResult, MetricDirection } from 'common/types/responses'
import {
AxisRange,
VariantIdentity,
Expand Down Expand Up @@ -31,10 +31,12 @@ type ExperimentResultsAxisChartProps = {
identities: VariantIdentity[]
metricName: string
metricResult?: BayesianMetricResult
direction: MetricDirection
range: AxisRange
}

const ExperimentResultsAxisChart: FC<ExperimentResultsAxisChartProps> = ({
direction,
identities,
metricName,
metricResult,
Expand Down Expand Up @@ -87,7 +89,7 @@ const ExperimentResultsAxisChart: FC<ExperimentResultsAxisChartProps> = ({
)
}
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ExperimentMetric,
Inference,
MetricAggregation,
MetricDirection,
VariantStats,
} from 'common/types/responses'
import {
Expand Down Expand Up @@ -43,6 +44,7 @@ const renderMetricValue = (
const renderLift = (
identity: VariantIdentity,
inference: Inference | null,
direction: MetricDirection,
liftRange: number,
): ReactNode => {
if (identity.isControl) {
Expand All @@ -51,7 +53,7 @@ const renderLift = (
if (!inference) {
return <span className='text-secondary fs-caption'>Collecting data…</span>
}
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)
Expand Down Expand Up @@ -195,7 +197,14 @@ const ExperimentResultsScorecardTable: FC<
</td>
<td>{stats ? stats.n.toLocaleString() : '—'}</td>
<td>{renderMetricValue(stats, metric.aggregation)}</td>
<td>{renderLift(v, inference, liftRange)}</td>
<td>
{renderLift(
v,
inference,
metric.direction ?? 'up',
liftRange,
)}
</td>
<td>{renderCI(v, inference)}</td>
<td>
{renderWinProbability(v, inference, v.key === winnerKey)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ const ExperimentSummaryScorecard: FC<ExperimentSummaryScorecardProps> = ({
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 (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ import {
computeLiftRange,
computeAxisRange,
buildExposuresChartData,
LiftTone,
deriveSummary,
formatBucketLabel,
getHeadlineTotal,
getVariantIdentities,
getVariantTotals,
getWinningVariant,
isLiftFavourable,
liftToPercent,
valueToPercent,
} from 'components/experiments/results/derive'
Expand All @@ -19,6 +21,7 @@ import {
Experiment,
ExperimentFeature,
ExposuresSummary,
MetricDirection,
MultivariateOption,
} from 'common/types/responses'

Expand Down Expand Up @@ -300,6 +303,7 @@ describe('deriveSummary', () => {
{
aggregation: 'occurrence',
created_at: '2026-06-01T00:00:00Z',
direction: 'up',
expected_direction: 'increase',
id: 1,
metric: 1,
Expand All @@ -320,7 +324,7 @@ describe('deriveSummary', () => {
expect(deriveSummary(experiment, results(metricResult))).toMatchObject({
chanceToBest: '75%',
controlWins: false,
liftFavourable: true,
liftTone: 'success',
liftVsControl: '+8.0%',
winnerName: 'b',
})
Expand All @@ -332,11 +336,41 @@ describe('deriveSummary', () => {
).toMatchObject({
chanceToBest: '85%',
controlWins: true,
liftFavourable: false,
liftTone: 'neutral',
liftVsControl: 'Baseline',
winnerName: 'Control',
})
})

it.each<[MetricDirection | undefined, LiftTone]>([
['up', 'success'],
['down', 'danger'],
['informational', 'neutral'],
[undefined, 'success'], // legacy payloads without direction default to up
Comment on lines +345 to +349

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Extract the optional direction union into a named type.

MetricDirection | undefined is declared inline in the it.each generic. Define a named alias, such as OptionalMetricDirection, and use it here.

As per coding guidelines, TypeScript union types in frontend/**/*.{ts,tsx} must be extracted into named types.

Proposed adjustment
+type OptionalMetricDirection = MetricDirection | undefined
+
-it.each<[MetricDirection | undefined, LiftTone]>([
+it.each<[OptionalMetricDirection, LiftTone]>([
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it.each<[MetricDirection | undefined, LiftTone]>([
['up', 'success'],
['down', 'danger'],
['informational', 'neutral'],
[undefined, 'success'], // legacy payloads without direction default to up
type OptionalMetricDirection = MetricDirection | undefined
it.each<[OptionalMetricDirection, LiftTone]>([
['up', 'success'],
['down', 'danger'],
['informational', 'neutral'],
[undefined, 'success'], // legacy payloads without direction default to up

Source: Coding guidelines

])(
'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('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', () => {
Expand Down
44 changes: 33 additions & 11 deletions frontend/web/components/experiments/results/derive.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -9,6 +13,7 @@ import {
ExposureGranularity,
ExposuresSummary,
Inference,
MetricDirection,
MultivariateOption,
} from 'common/types/responses'
import { getPrimaryMetric } from 'components/experiments/constants'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -185,14 +199,16 @@ export const getWinningVariant = (
return best
}

export type LiftTone = 'success' | 'danger' | 'neutral'

export type SummaryStats = {
winnerName: string
winnerColour: string
controlColour: string
controlWins: boolean
chanceToBest: string
liftVsControl: string
liftFavourable: boolean
liftTone: LiftTone
}

export const deriveSummary = (
Expand All @@ -211,13 +227,19 @@ 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 && direction !== 'informational') {
liftTone = isLiftFavourable(winner.inference.lift, 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',
Expand Down
Loading