Parked: fix(experimentation): test SRM against the rollout override's split by Zaimwa9 · Pull Request #8798 · Flagsmith/flagsmith

Zaimwa9 · 2026-10-16T09:25:16Z

Thanks for submitting a PR! Please check the boxes below:

I have read the Contributing Guide.
I have added information to docs/ if required so people know about the feature.
I have filled in the "Changes" section below.
I have filled in the "How did you test this code" section below.

Changes

srm_p_value was null for every experiment: expected variant shares were read from the environment default, which starting an experiment resets to 0% (all control), so the chi-square test bailed out.
Expected shares now come from the experiment's rollout override split.
Effect: the "Sample ratio mismatch detected" caption under the scorecard can now actually show.

How did you test this code?

Unit tests in tests/unit/experimentation/test_services.py; experimentation suite passes (757).

…llout-split

vercel · 2026-10-16T09:25:18Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
docs	Ready	Preview	Oct 16, 2026 9:26am UTC

2 Skipped Deployments

Project	Deployment	Actions	Updated
flagsmith-frontend-preview	Ignored	Preview	Oct 16, 2026 9:26am UTC
flagsmith-frontend-staging	Ignored	Preview	Oct 16, 2026 9:26am UTC

coderabbitai · 2026-10-16T09:25:26Z

📝 Walkthrough

Expected variant shares now use the experiment’s live rollout allocation. The calculation returns no shares when no rollout exists, excludes zero-weight variants and uses the unallocated remainder for control when the total is below 100%. Tests cover rollout allocation cases. The event catalogue updates source line references for eight events.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🟡 Moderate · up to bb65e

The sample ratio mismatch check now uses the live rollout split. If a split changes mid-experiment, it can raise false alerts. Traffic reaching a variant with zero weight is ignored, so a real mismatch can go unreported. Address both before merging.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Zaimwa9 · 2026-10-16T09:30:33Z

@themis-blindfold review

codecov · 2026-10-16T09:31:03Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.87%. Comparing base (abb07d8) to head (bb65e7e).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8798   +/-   ##
=======================================
  Coverage   98.87%   98.87%           
=======================================
  Files        1678     1678           
  Lines       69561    69574   +13     
=======================================
+ Hits        68775    68788   +13     
  Misses        786      786

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 2

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: cb908b70-0bd2-4960-8ef9-fa455bc0cc41

📥 Commits

Reviewing files that changed from the base of the PR and between b6d6988 and bb65e7e.

📒 Files selected for processing: 3

api/experimentation/services.py
api/tests/unit/experimentation/test_services.py
docs/docs/deployment-self-hosting/observability/_events-catalogue.md

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.

coderabbitai · 2026-10-16T09:35:39Z

+            additional_filters=Q(
+                feature_segment__segment_id=experiment.rollout_segment_id,
+                identity__isnull=True,
+            ),


🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Account for split changes within the results window.

If a running experiment changes its variant weights, this query returns only the current split. build_results_summary compares that split with exposures aggregated across the results window. Exposures assigned under the earlier split can therefore produce a false SRM alert. Use allocation history for the exposure window, or restrict the check to exposures collected under one stable split.

coderabbitai · 2026-10-16T09:35:39Z

+        if mv_value.percentage_allocation > 0:
+            # A zero share has no expected traffic to test against.
+            shares[key] = mv_value.percentage_allocation / 100


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

rg -n 'expected_shares|srm_p_value' api/experimentation/services.py git diff b6d69889652df199a21dfa9f64a2c8a7550936f0 bb65e7e3e3d7e565e70dcf30058e3e5cffd3647d -- api/experimentation/services.py

Repository: Flagsmith/flagsmith

Length of output: 3006

🏁 Script executed:

set -eu printf '%s\n' '--- current services.py relevant block ---' nl -ba api/experimentation/services.py | sed -n '250,420p' printf '%s\n' '--- merge-base services.py relevant block ---' git show b6d69889652df199a21dfa9f64a2c8a7550936f0:api/experimentation/services.py | nl -ba | sed -n '250,420p' printf '%s\n' '--- srm_p_value declaration and implementation references ---' rg -n -F --glob '*.py' -e 'def srm_p_value' -e 'srm_p_value(' . printf '%s\n' '--- exposure_counts construction and relevant tests ---' rg -n -F --glob '*.py' -e 'exposure_counts' -e 'expected_shares' -e 'Sample ratio mismatch' tests api

Repository: Flagsmith/flagsmith

Length of output: 21276

🏁 Script executed:

set -eu printf '%s\n' '--- srm_p_value implementation ---' nl -ba api/experimentation/stats.py | sed -n '80,115p' printf '%s\n' '--- focused build_results_summary tests ---' nl -ba api/tests/unit/experimentation/test_services.py | sed -n '1160,1355p' printf '%s\n' '--- exposure aggregation implementation ---' nl -ba api/experimentation/results_query.py | sed -n '285,325p'

Repository: Flagsmith/flagsmith

Length of output: 11975

Flag exposures for variants outside expected_shares.

When a zero-weight variant has exposures, _expected_variant_shares excludes it. build_results_summary then omits its count from srm_p_value, which can hide an SRM mismatch.

Do not restore a zero share directly. srm_p_value returns None for non-positive expected shares. Detect positive exposure counts for keys absent from expected_shares and report a mismatch instead.

Suggested fix

if expected_shares and total >= SRM_MIN_TOTAL_IDENTITIES: - srm = srm_p_value( - [exposure_counts.get(variant, 0) for variant in expected_shares], - list(expected_shares.values()), - ) + has_unexpected_exposures = any( + count > 0 + for variant, count in exposure_counts.items() + if variant not in expected_shares + ) + if has_unexpected_exposures: + srm = 0.0 + else: + srm = srm_p_value( + [exposure_counts.get(variant, 0) for variant in expected_shares], + list(expected_shares.values()), + )

Zaimwa9 added 2 commits October 15, 2026 18:19

fix(experimentation): test SRM against the rollout override's split

4b3e9cc

Merge remote-tracking branch 'origin/main' into fix/experiment-srm-ro…

a956289

…llout-split

github-actions Bot added api Issue related to the REST API fix labels Oct 16, 2026

vercel Bot deployed to Preview – docs October 16, 2026 09:25

View deployment

vercel Bot deployed to Preview – flagsmith-frontend-staging October 16, 2026 09:25

View deployment

vercel Bot deployed to Preview – flagsmith-frontend-preview October 16, 2026 09:25

View deployment

chore: Update documentation artefacts

bb65e7e

github-actions Bot added the docs Documentation updates label Oct 16, 2026

vercel Bot deployed to Preview – docs October 16, 2026 09:26

View deployment

coderabbitai Bot reviewed Oct 16, 2026

View reviewed changes

Zaimwa9 changed the title ~~fix(experimentation): test SRM against the rollout override's split~~ Parked: fix(experimentation): test SRM against the rollout override's split Oct 16, 2026

Navigation Menu

Repository navigation

Parked: fix(experimentation): test SRM against the rollout override's split - #8798

Parked: fix(experimentation): test SRM against the rollout override's split#8798
Zaimwa9 wants to merge 3 commits into
mainfrom
fix/experiment-srm-rollout-split

Zaimwa9 commented Oct 16, 2026

Uh oh!

vercel Bot commented Oct 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 16, 2026 •

edited

Loading

Uh oh!

Zaimwa9 commented Oct 16, 2026

Uh oh!

codecov Bot commented Oct 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Oct 16, 2026

Uh oh!

coderabbitai Bot Oct 16, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Zaimwa9 commented Oct 16, 2026

Changes

How did you test this code?

Uh oh!

vercel Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zaimwa9 commented Oct 16, 2026

Uh oh!

codecov Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Oct 16, 2026 •

edited

Loading

coderabbitai Bot commented Oct 16, 2026 •

edited

Loading

codecov Bot commented Oct 16, 2026 •

edited

Loading