[AI-FSSDK] [FSSDK-12369] Add local holdouts support#628
Conversation
jaeopt
left a comment
jaeopt
left a comment
There was a problem hiding this comment.
LGTM - a couple of comments to confirm
| // Evaluate global holdouts at flag level (before any rules are iterated) | ||
| List<Holdout> globalHoldouts = projectConfig.getGlobalHoldouts(); | ||
| if (!globalHoldouts.isEmpty()) { | ||
| for (Holdout holdout : globalHoldouts) { |
There was a problem hiding this comment.
This is logically correct - we check all global holdouts for each flag.
A concern is when we run decideAll, we repeat this same for all flags, wasting computation. It can be a big wast for 100+ flags. We can consider -
- move this global to outside of the flags loop
- if hit in any global HO, return an array of the same HO variation - OFF
| Experiment rolloutRule = rollout.getExperiments().get(index); | ||
|
|
||
| // Step 1: Check forced decision for this delivery rule (highest priority). | ||
| String rolloutRuleKey = rolloutRule.getKey(); |
There was a problem hiding this comment.
We here add Forced-Decision check for rollouts.
This is a fix for not-related to HO, but we can keep it since this is good and consistent for all SDKs.
jaeopt
left a comment
jaeopt
left a comment
There was a problem hiding this comment.
Found ForcedDecision repeated 2 times for each rule.
| if (experiment != null) { | ||
| String ruleKey = experiment.getKey(); | ||
| OptimizelyDecisionContext fdContext = new OptimizelyDecisionContext(flagKey, ruleKey); | ||
| DecisionResponse<Variation> fdResponse = validatedForcedDecision(fdContext, projectConfig, user); |
There was a problem hiding this comment.
We already have forced-decision check in getVariationFromExperimentRule(), so redundant.
We should remove one of them.
… in DecisionService Move validatedForcedDecision and localHoldout checks from getVariationFromExperiment and getVariationForFeatureInRollout into getVariationFromExperimentRule and getVariationFromDeliveryRule, eliminating redundant evaluation. Extract shared local holdout logic into evaluateLocalHoldouts() and add unit tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Summary
Adds local holdouts support to the Java SDK.
Jira ticket: FSSDK-12369
Changes
Test plan
🤖 Generated with Claude Code via AI-FSSDK