Context
After tracing how ABTesting decides whether a variant wins, I think the confidence estimate in _calculate_significance produces numbers that don't mean what callers will assume they mean.
The code
src/evolution/ab_testing.py:549-558
# 简化的显著性判断
# 实际应用中应使用更严格的统计检验
min_samples = min(control_data.impressions, variant_data.impressions)
is_significant = abs(lift) > 0.05 and min_samples >= 100
# 计算置信度(简化版)
confidence = 0
if min_samples >= 100:
# 使用样本量和效果大小估算置信度
confidence = min(0.99, min_samples / 1000 * (1 + abs(lift)))
Why this is wrong
The formula min_samples / 1000 * (1 + abs(lift)) has no grounding in the statistics of proportions, and it self-contradicts the is_significant flag computed two lines above:
- A result that just clears the significance bar (
min_samples == 100, lift == 0.06) reports confidence = 100/1000 * 1.06 ≈ 0.106. So the same comparison is simultaneously labelled "significant" and "~11% confidence."
- Confidence scales linearly and unboundedly with sample size: at
min_samples == 1000 and zero lift you already hit 1.0 (clamped to 0.99), regardless of how noisy the conversion rates are.
- It scales with
(1 + abs(lift)) even when the absolute conversion counts are tiny, so a large relative lift on a handful of conversions looks more confident than it should.
The author already flagged this as "简化版 / simplified" — this issue is to make the simplification at least directionally correct, because downstream code and humans will read confidence as a probability.
Suggested direction
Use the standard two-proportion z-test and derive confidence from the p-value, e.g.:
from math import sqrt
from statistics import NormalDist
p1, n1 = control_rate, control_data.impressions
p2, n2 = variant_rate, variant_data.impressions
p_pool = (control_data.conversions + variant_data.conversions) / (n1 + n2)
se = sqrt(p_pool * (1 - p_pool) * (1 / n1 + 1 / n2))
z = (p2 - p1) / se if se > 0 else 0.0
confidence = 2 * NormalDist().cdf(abs(z)) - 1 # two-sided
is_significant = confidence >= 0.95
That ties is_significant and confidence to the same underlying test instead of two unrelated heuristics.
Severity: Medium (results are usable directionally but the reported confidence number is misleading for decision-making)
Context
After tracing how
ABTestingdecides whether a variant wins, I think the confidence estimate in_calculate_significanceproduces numbers that don't mean what callers will assume they mean.The code
src/evolution/ab_testing.py:549-558Why this is wrong
The formula
min_samples / 1000 * (1 + abs(lift))has no grounding in the statistics of proportions, and it self-contradicts theis_significantflag computed two lines above:min_samples == 100,lift == 0.06) reportsconfidence = 100/1000 * 1.06 ≈ 0.106. So the same comparison is simultaneously labelled "significant" and "~11% confidence."min_samples == 1000and zero lift you already hit1.0(clamped to0.99), regardless of how noisy the conversion rates are.(1 + abs(lift))even when the absolute conversion counts are tiny, so a large relative lift on a handful of conversions looks more confident than it should.The author already flagged this as "简化版 / simplified" — this issue is to make the simplification at least directionally correct, because downstream code and humans will read
confidenceas a probability.Suggested direction
Use the standard two-proportion z-test and derive confidence from the p-value, e.g.:
That ties
is_significantandconfidenceto the same underlying test instead of two unrelated heuristics.Severity: Medium (results are usable directionally but the reported confidence number is misleading for decision-making)