[AI-FSSDK] [FSSDK-12369] Add local holdouts support to Go SDK#451
Open
Mat001 wants to merge 3 commits into
Open
[AI-FSSDK] [FSSDK-12369] Add local holdouts support to Go SDK#451Mat001 wants to merge 3 commits into
Mat001 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds local holdout support so holdouts can target specific experiment or rollout rules while preserving existing global holdout behavior for datafiles without includedRules.
Changes:
- Adds
IncludedRulesand global/local holdout classification. - Indexes local holdouts by rule ID in project config mapping.
- Evaluates local holdouts in feature experiment and rollout decision flows after forced decisions and before normal rule evaluation.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/entities/experiment.go | Adds local/global holdout metadata helper. |
| pkg/config/interface.go | Extends project config holdout access APIs. |
| pkg/config/datafileprojectconfig/entities/entities.go | Adds datafile includedRules parsing field. |
| pkg/config/datafileprojectconfig/config.go | Stores local and global holdout indexes. |
| pkg/config/datafileprojectconfig/mappers/holdout.go | Maps global holdouts by flag and local holdouts by rule. |
| pkg/config/datafileprojectconfig/mappers/holdout_test.go | Adds mapper coverage for global/local holdout behavior. |
| pkg/decision/holdout_service.go | Adds rule-level local holdout evaluation. |
| pkg/decision/holdout_service_test.go | Adds local holdout decision service tests. |
| pkg/decision/feature_experiment_service.go | Integrates local holdout checks into feature experiment evaluation. |
| pkg/decision/feature_experiment_service_test.go | Updates constructor tests and forced-decision precedence coverage. |
| pkg/decision/rollout_service.go | Integrates local holdout checks into rollout rule evaluation. |
| pkg/decision/composite_feature_service.go | Shares holdout service with feature experiment service. |
| pkg/decision/helpers_test.go | Updates decision test mock config interface. |
| pkg/decision/evaluator/audience_evaluator_test.go | Updates evaluator test mock config interface. |
| pkg/cmab/service_test.go | Updates CMAB test mock config interface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mat001
force-pushed
the
ai/mat001/FSSDK-12369-local-holdouts
branch
from
a4fed68 to
9e39ee2
Compare
- Add rollout service tests for local holdout in regular and fallback rules, and FD-beats-local-holdout ordering enforcement - Add feature experiment service FD-beats-100%-local-holdout enforcement test - Add GetHoldoutsForRule config tests (previously untested) - Extract checkForLocalHoldout closure to getLocalHoldoutDecision method to bring GetDecision cyclomatic complexity from 18 to 16 (lint fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
muzahidul-opti
requested changes
May 23, 2026
muzahidul-opti
left a comment
muzahidul-opti
left a comment
There was a problem hiding this comment.
Looks good to me. Added some comments.
| Variations []Variation `json:"variations"` | ||
| TrafficAllocation []TrafficAllocation `json:"trafficAllocation"` | ||
| // IncludedRules is optional. nil = global holdout (applies to all rules across all flags). | ||
| // Non-nil array = local holdout (applies only to the specified rule IDs). |
There was a problem hiding this comment.
Looks consistence with other sdks
| if len(runningHoldouts) > 0 { | ||
| flagHoldoutsMap[feature.Key] = runningHoldouts | ||
| if len(globalHoldouts) > 0 { | ||
| flagHoldoutsMap[feature.Key] = globalHoldouts |
There was a problem hiding this comment.
Do we still need flagHoldoutsMap? I think ruleHoldoutsMap is enough for targeting local HO
| @@ -63,6 +63,10 @@ type DatafileProjectConfig struct { | |||
| holdouts []entities.Holdout | |||
| holdoutIDMap map[string]entities.Holdout | |||
| flagHoldoutsMap map[string][]entities.Holdout | |||
There was a problem hiding this comment.
I think we can remove it, no longer needed.
| } | ||
|
|
||
| // GetHoldoutsForFlag returns all holdouts applicable to the given feature flag | ||
| // GetHoldoutsForFlag returns all global holdouts applicable to the given feature flag. |
There was a problem hiding this comment.
I think we no longer need flag base method anymore.
| audienceMap, audienceSegmentList := mappers.MapAudiences(append(datafile.TypedAudiences, datafile.Audiences...)) | ||
| flagVariationsMap := mappers.MapFlagVariations(featureMap) | ||
| holdouts, holdoutIDMap, flagHoldoutsMap := mappers.MapHoldouts(datafile.Holdouts, featureMap) | ||
| holdouts, holdoutIDMap, flagHoldoutsMap, ruleHoldoutsMap := mappers.MapHoldouts(datafile.Holdouts, featureMap) |
| @@ -57,6 +57,12 @@ type ProjectConfig interface { | |||
| GetFlagVariationsMap() map[string][]entities.Variation | |||
| GetRegion() string | |||
| GetHoldoutsForFlag(featureKey string) []entities.Holdout | |||
| // GetDecisionForRule evaluates local holdouts targeting the given rule ID. | ||
| // It returns a non-empty FeatureDecision if the user is bucketed into a local holdout for this rule. | ||
| // Local holdouts are evaluated per-rule, after forced decisions, before audience/traffic checks. | ||
| func (h HoldoutService) GetDecisionForRule(ruleID string, projectConfig config.ProjectConfig, userContext entities.UserContext, options *decide.Options) (FeatureDecision, decide.DecisionReasons, error) { |
There was a problem hiding this comment.
Can we update the methods name as like:
- GetGlobalDecision
- GetLocalDecisionForRule
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds local holdouts support to the Go SDK. Local holdouts allow targeting specific experiment and delivery rules within a flag, while global holdouts continue to apply across all rules in all flags. This introduces a rule-level targeting system using an optional
includedRulesfield on the holdout data model.Changes
IncludedRules *[]stringfield to theHoldoutentity withIsGlobal()helper methodHoldoutConfigwithGetGlobalHoldouts()andGetHoldoutsForRule(ruleID)APIs and updated the holdout mapper to build the rule-to-holdout indexFeatureExperimentServiceandRolloutService, evaluated after forced decisions and before audience/traffic checksGetDecisionForRulemethod toHoldoutServicefor local holdout bucketing logicincludedRulesdefault to global holdout behaviorJira Ticket
FSSDK-12369