feat(experimentation): targeted audience for experiment rollouts - #8472
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughExperiment rollouts now accept audience segments with Priority: ➖ Normal — Schedule the targeted experiment-audience change because it spans rollout APIs, audience rule compilation, persistence, validation, and cohort deletion safeguards across experimentation. Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🔵 Low · up to Audience targeting adds compiled nested segment rules to experiment rollouts. Deeply nested rules may cause elevated query volume when rollout configuration is loaded, so this should be confirmed before high-volume use. 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. Comment |
4f33cff to
8faabc4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8472 +/- ##
========================================
Coverage 98.81% 98.82%
========================================
Files 1621 1622 +1
Lines 66299 66708 +409
========================================
+ Hits 65514 65923 +409
Misses 785 785 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
182c216 to
aa61c41
Compare
4a2ec7c to
77234df
Compare
9695d3b to
79f40a9
Compare
5932fc6 to
ed13898
Compare
f39e6d5 to
d0bd8bc
Compare
…read rules off the rollout segment
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/projects/services.py (1)
29-35: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winPrefetch deepest-level conditions in all rule-loading paths
_compile_audience()can produce rules at the fifth nested level, andmap_segment_rule_to_engine()recursively readsrule.conditions.all(). Addrules__rules__rules__rules__rules__conditionsto the prefetch lists inapi/projects/services.py,EnvironmentManager.filter_for_document_builder(), andEnvironment.get_segments_from_cache(). Otherwise, environment-document generation can issue one lazy query per deepest rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: b3fc7fac-2dfc-4463-a434-f4fac3d7479d
📒 Files selected for processing (9)
api/cohorts/services.pyapi/experimentation/dataclasses.pyapi/experimentation/migrations/0013_add_experiment_audience.pyapi/experimentation/models.pyapi/experimentation/services.pyapi/tests/unit/cohorts/test_services.pyapi/tests/unit/experimentation/test_experiment_views.pyapi/tests/unit/experimentation/test_services.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.md
💤 Files with no reviewable changes (1)
- api/experimentation/dataclasses.py
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
…message to constants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Experiment rollouts can now target an audience:
Segment > % rollout > % variant weights.audience: {"match": "any"|"all", "segment_ids": [...]}(omitted = unchanged).experiment-{id}-rolloutsystem segment, so later segment edits don't affect a running experiment. No engine, SDK, or environment-document schema changes.Experiment.audienceJSONField (migration0013); the audience is immutable once the experiment starts.PERCENTAGE_SPLITrules, no rules, duplicates, over the cap).experimentation.rollout.appliedstructlog event on rollout changes.MAX_AUDIENCE_SEGMENTSis 1 for now. The multi-segment machinery is implemented and tested — theany/allcombinator, the per-segment rule wrappers, the compile and enrolment paths — but the cap holds it to a single segment until the Java SDK is fixed:Warning
A multi-segment
anyaudience compiles to anANYrule with nested sub-rules. The released Java SDK (flagsmith-java-client≤ 8.1.0) ANDs sub-rules regardless of the rule type in local evaluation, so such an audience would evaluate asalland under-fill the experiment — silently, with no SRM signal. Raising the cap needs the Java engine to respect the rule type for sub-rules first.Raising the cap also grows the environment document, since each targeted segment's rules are copied into the rollout segment (cohort segments compile to a single system-trait condition). A source segment with one top-level rule is inlined rather than wrapped, keeping the common case within the document builder's prefetch depth and the org exporter's two-level rule selection.
The dashboard UI comes in a stacked follow-up PR.
How did you test this code?
Unit and integration tests, 100% diff coverage. Multi-segment compilation and enrolment stay covered via a fixture that lifts the cap, so the behaviour is tested ahead of the cap being raised. A query-count regression test pins the environment-document cost at a fixed number of prefetches for a nested audience.
Manually on a local stack: created a segment + seeded identities, started an experiment targeting it at 50% rollout with a 50/50 split — ~50% of in-segment identities enrolled, none out-of-segment. Edited the source segment mid-run: enrolment unchanged (frozen copy). PATCHing a different audience after start returns 400.