Uh oh!
There was an error while loading. Please reload this page.
[AI-FSSDK] [FSSDK-12337] Add Feature Rollout support - #499
Conversation
- Move feature rollout tests from standalone test_feature_rollout.py into test_config.py following module-level testing convention - Use base.BaseTest instead of unittest.TestCase for consistency - Fix mypy strict type errors in Variation construction using cast - All checks pass: ruff, mypy --strict, pytest (941/941) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Inline _get_everyone_else_variation logic, remove unnecessary static method - Use get_rollout_from_id() to match TDD pseudocode - Remove isinstance check (rollout experiments are always dicts) - Remove 3 unit tests for deleted helper method (edge cases already covered by integration-level tests) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Restore as instance method matching TDD pseudocode structure - Takes flag (FeatureFlag) param, calls get_rollout_from_id internally - Caller simplified to: everyone_else_variation = self._get_everyone_else_variation(flag)
- Add ExperimentTypes enum in helpers/enums.py with AB, MAB, CMAB, FEATURE_ROLLOUT - Use ExperimentTypes.FEATURE_ROLLOUT constant in config parsing instead of raw string
- Add ExperimentType Literal type in helpers/types.py: 'a/b', 'mab', 'cmab', 'feature_rollout' - Change Experiment.type from Optional[str] to Optional[ExperimentType]
…check - Remove redundant ExperimentTypes class from enums.py (ExperimentType Literal suffices) - Simplify getattr(experiment, 'type', None) to experiment.type
| # Feature Rollout support: inject the "everyone else" variation | ||
| # into any experiment with type == "feature_rollout" | ||
| everyone_else_variation = self._get_everyone_else_variation(feature) | ||
| if everyone_else_variation is not None: |
There was a problem hiding this comment.
We need this condition to the ticket prompt -
if everyone_else is null, we need handle specially. Let's discuss.
| var_entity = entities.Variation( | ||
| id=everyone_else_variation['id'], | ||
| key=everyone_else_variation['key'], | ||
| featureEnabled=bool(everyone_else_variation.get('featureEnabled', False)), | ||
| variables=cast( | ||
| Optional[list[entities.Variable]], | ||
| everyone_else_variation.get('variables'), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Can this be returned by _get_everyone_else_variation() instead of building redeatedly?
We can try add this to the prompt.
…lse_variation - Build Variation entity once in helper, derive dict from it in caller - Addresses PR review comment from jaeopt
Removed 7 tests that were covered by other tests: - test_experiment_type_field_parsed (covered by injection test) - test_feature_rollout_with_empty_rollout_experiments (similar to no_rollout) - test_feature_rollout_multiple_experiments_mixed_types (covered by injection + unchanged) - test_feature_rollout_flag_variations_map_includes_injected (subset of maps test) - test_experiment_type_ab (just string assignment) - test_feature_rollout_with_variables_on_everyone_else (edge case) - test_existing_datafile_not_broken (covered by none_when_missing + unchanged)
| if not variations: | ||
| return None | ||
| variation_dict = variations[0] |
There was a problem hiding this comment.
Is it save to access? What happen if it is empty
There was a problem hiding this comment.
It is safe. Line 722-723 ensures its not empty and if empty None is returned.
jaeopt
left a comment
There was a problem hiding this comment.
Looks good. A couple of changes suggested.
| trafficAllocation: int | ||
| ExperimentType = Literal['a/b', 'mab', 'cmab', 'feature_rollout'] |
There was a problem hiding this comment.
flags have these types -
class RuleTypes(enum.Enum):
targeted_delivery = "targeted_delivery"
ab = "a/b"
multi_armed_bandit = "multi_armed_bandit"
contextual_multi_armed_bandit = "contextual_multi_armed_bandit"
if we want to keep short names - let's consistent. what about
{"ab", "mab", "cmab", "td", "fr"}
There was a problem hiding this comment.
We can specify these types in the ticket.
Also need to update TDD as well.
Uh oh!
There was an error while loading. Please reload this page.
…livery type - Add ExperimentTypes class in enums.py with ab, mab, cmab, td, fr - Add 'targeted_delivery' to ExperimentType Literal in types.py - Use enums.ExperimentTypes.fr constant in injection check - Add test for type field parsing from datafile
Remove test_feature_rollout_everyone_else_is_last_rollout_rule to match updated Jira ticket test requirements.
Update ExperimentTypes and ExperimentType Literal to use actual backend values: 'multi_armed_bandit' and 'contextual_multi_armed_bandit' instead of shorthand 'mab' and 'cmab'.
esrakartalOpt
left a comment
There was a problem hiding this comment.
All comments are implemented. Looks good to me 👍
Uh oh!
There was an error while loading. Please reload this page.
Summary
Adds Feature Rollout support to the Python SDK. Feature Rollouts are a new experiment rule type that combines Targeted Delivery simplicity with A/B test measurement capabilities. During project config parsing, the "everyone else" variation from the flag's rollout is injected into any experiment with type "feature_rollout", enabling correct evaluation without changes to decision logic.
Changes
typestring field to the Experiment data model_get_everyone_else_variationhelper method to extract the last rollout rule's first variationJira Ticket
FSSDK-12337