feat(experimentation): default to control when rollout segment is created#7905
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7905 +/- ##
========================================
Coverage 98.63% 98.64%
========================================
Files 1497 1501 +4
Lines 59261 59466 +205
========================================
+ Hits 58455 58660 +205
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8451662 to
8275f6d
Compare
…ated When a feature is first added to an experiment's rollout, zero every variant's allocation on the feature's environment-default feature state so control (the unallocated remainder) receives 100% of out-of-rollout traffic. The rollout segment override keeps the experiment's own split. This only runs when the rollout segment is first created; tuning the rollout of a running experiment leaves the default allocations alone.
8275f6d to
cf19595
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughExperiment rollout application now zeroes environment-default multivariate allocations when creating the first rollout segment, while preserving the rollout segment’s submitted allocation split. Existing rollout updates leave manually edited default allocations unchanged. Feature-state value serialisation is centralised and typed, including boolean formatting. Unit and view tests cover standard and versioned environment paths, and warehouse event documentation references are updated. Estimated code review effort: 3 (Moderate) | ~25 minutes Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Zaimwa9
left a comment
There was a problem hiding this comment.
Looks good overall but we should fix the null => "None" before moving forward
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/experimentation/services.py (1)
683-687: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRe-check the experiment status after acquiring the lock.
The
COMPLETEDguard runs beforerefresh_from_db(). If another transaction completes the experiment between these operations, this transaction can still write a rollout for a completed experiment. Repeat the guard immediately after refreshing the locked row.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6d63392d-3b95-4df3-bacb-9b95934e624b
📒 Files selected for processing (3)
api/experimentation/services.pyapi/tests/unit/experimentation/test_services.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.md
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
When a feature is first added to an experiment's rollout — the moment the rollout system segment is created — zero every variant's allocation on the environment-default feature state, so identities outside the rollout cohort receive control (the unallocated remainder) while the experiment runs.
update_flagpath, working under both v1 and v2 versioning.How did you test this code?
Unit tests (
make test): service tests covering first-rollout zeroing and later updates leaving the default alone; view tests asserting create-with-rollout zeroes the default and preserves the override split under both v1 and v2 versioning.mypy,ruff, andflagsmith-lint-testsclean.