[AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit

pkg/client/client.go

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2019-2025, Optimizely, Inc. and contributors                   *
+ * Copyright 2019-2026, Optimizely, Inc. and contributors                   *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -248,27 +248,18 @@ func (o *OptimizelyClient) dispatchDecisionEvents(projectConfig config.ProjectCo
 		o.EventProcessor.ProcessEvent(ue)
 		eventSent = true
 	}
-	if o.sendHoldoutImpression(projectConfig, featureDecision, usrContext, key) {
-		eventSent = true
+	for _, hd := range featureDecision.HoldoutDecisions {
+		if hd.Variation == nil {
+			continue
+		}
+		if hue, ok := event.CreateImpressionUserEvent(projectConfig, hd.Experiment, hd.Variation, usrContext, key, hd.Experiment.Key, decision.Holdout, hd.Variation.FeatureEnabled, nil); ok {
+			o.EventProcessor.ProcessEvent(hue)
+			eventSent = true
+		}
 	}
 	return eventSent
 }
 
-// sendHoldoutImpression emits a holdout impression event when a holdout was bypassed for a
-// targeted delivery rule (the decision carries the bucketed holdout experiment/variation).
-// It returns true if an event was dispatched.
-func (o *OptimizelyClient) sendHoldoutImpression(projectConfig config.ProjectConfig, featureDecision decision.FeatureDecision, userContext entities.UserContext, key string) bool {
-	if featureDecision.HoldoutExperiment == nil || featureDecision.HoldoutVariation == nil {
-		return false
-	}
-	if hue, hok := event.CreateImpressionUserEvent(projectConfig, *featureDecision.HoldoutExperiment,
-		featureDecision.HoldoutVariation, userContext, key, featureDecision.HoldoutExperiment.Key, decision.Holdout, featureDecision.HoldoutVariation.FeatureEnabled, nil); hok {
-		o.EventProcessor.ProcessEvent(hue)
-		return true
-	}
-	return false
-}
-
 func (o *OptimizelyClient) decideForKeys(userContext OptimizelyUserContext, keys []string, options *decide.Options) map[string]OptimizelyDecision {
 	var err error
 	defer func() {
@@ -546,8 +537,6 @@ func (o *OptimizelyClient) IsFeatureEnabled(featureKey string, userContext entit
 		featureDecision.Variation, userContext, featureKey, featureDecision.Experiment.Key, featureDecision.Source, result, featureDecision.CmabUUID); ok && featureDecision.Source != "" {
 		o.EventProcessor.ProcessEvent(ue)
 	}
-	// Send holdout impression when holdout is bypassed for targeted delivery
-	o.sendHoldoutImpression(decisionContext.ProjectConfig, featureDecision, userContext, featureKey)
 
 	return result, err
 }
@@ -913,8 +902,6 @@ func (o *OptimizelyClient) GetDetailedFeatureDecisionUnsafe(featureKey string, u
 				featureDecision.Variation, userContext, featureKey, featureDecision.Experiment.Key, featureDecision.Source, decisionInfo.Enabled, featureDecision.CmabUUID); ok {
 				o.EventProcessor.ProcessEvent(ue)
 			}
-			// Send holdout impression when holdout is bypassed for targeted delivery
-			o.sendHoldoutImpression(decisionContext.ProjectConfig, featureDecision, userContext, featureKey)
 		}
 	}
 

pkg/client/client_test.go

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2019-2020,2022-2025 Optimizely, Inc. and contributors          *
+ * Copyright 2019-2020,2022-2026 Optimizely, Inc. and contributors          *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -3271,3 +3271,223 @@ func TestClientTestSuiteTrackEvent(t *testing.T) {
 func TestClientTestSuiteTrackNotification(t *testing.T) {
 	suite.Run(t, new(ClientTestSuiteTrackNotification))
 }
+
+type localHoldoutClientFixture struct {
+	client         *OptimizelyClient
+	config         *MockProjectConfig
+	feature        entities.Feature
+	eventProcessor *MockEventProcessor
+}
+
+func newLocalHoldoutClientFixture(featureDecision decision.FeatureDecision) localHoldoutClientFixture {
+	mockConfig := new(MockProjectConfig)
+	mockConfigManager := new(MockProjectConfigManager)
+	mockDecisionService := new(MockDecisionService)
+	mockEventProcessor := new(MockEventProcessor)
+
+	feature := makeTestFeatureWithExperiment("feature_1", makeTestExperiment("exp_1"))
+	mockConfig.On("GetFeatureByKey", feature.Key).Return(feature, nil)
+	mockConfigManager.On("GetConfig").Return(mockConfig, nil)
+	mockDecisionService.On("GetFeatureDecision", mock.Anything, mock.Anything, mock.Anything).Return(featureDecision, decide.NewDecisionReasons(nil), nil)
+	mockEventProcessor.On("ProcessEvent", mock.AnythingOfType("event.UserEvent"))
+
+	return localHoldoutClientFixture{
+		client: &OptimizelyClient{
+			ConfigManager:        mockConfigManager,
+			DecisionService:      mockDecisionService,
+			EventProcessor:       mockEventProcessor,
+			defaultDecideOptions: &decide.Options{},
+			logger:               logging.GetLogger("", ""),
+			tracer:               &MockTracer{},
+		},
+		config:         mockConfig,
+		feature:        feature,
+		eventProcessor: mockEventProcessor,
+	}
+}
+
+func makeLocalHoldoutDecision(holdoutKey string) decision.FeatureDecision {
+	variation := makeTestVariation("holdout_off", false)
+	return decision.FeatureDecision{
+		Experiment: makeTestExperimentWithVariations(holdoutKey, []entities.Variation{variation}),
+		Variation:  &variation,
+		Source:     decision.Holdout,
+	}
+}
+
+func processedImpressions(ep *MockEventProcessor, ruleType string) []event.UserEvent {
+	var events []event.UserEvent
+	for _, call := range ep.Calls {
+		if call.Method != "ProcessEvent" {
+			continue
+		}
+		ue := call.Arguments.Get(0).(event.UserEvent)
+		if ue.Impression != nil && ue.Impression.Metadata.RuleType == ruleType {
+			events = append(events, ue)
+		}
+	}
+	return events
+}
+
+func TestDecideLocalHoldoutHitThenLaterRuleServes(t *testing.T) {
+	servedVariation := makeTestVariation("on", true)
+	servedExperiment := makeTestExperimentWithVariations("exp_2", []entities.Variation{servedVariation})
+	f := newLocalHoldoutClientFixture(decision.FeatureDecision{
+		Experiment:       servedExperiment,
+		Variation:        &servedVariation,
+		Source:           decision.FeatureTest,
+		HoldoutDecisions: []decision.FeatureDecision{makeLocalHoldoutDecision("local_holdout")},
+	})
+	userContext := newOptimizelyUserContext(f.client, "test_user_1", nil, nil, nil)
+
+	d := f.client.decide(&userContext, f.feature.Key, &decide.Options{})
+
+	assert.Equal(t, "on", d.VariationKey)
+	assert.Equal(t, "exp_2", d.RuleKey)
+	assert.True(t, d.Enabled)
+	holdoutEvents := processedImpressions(f.eventProcessor, decision.Holdout)
+	assert.Len(t, holdoutEvents, 1)
+	assert.Equal(t, "local_holdout", holdoutEvents[0].Impression.Metadata.RuleKey)
+	assert.Equal(t, f.feature.Key, holdoutEvents[0].Impression.Metadata.FlagKey)
+	assert.Len(t, processedImpressions(f.eventProcessor, decision.FeatureTest), 1)
+}
+
+func TestDecideLocalHoldoutHitsOnSeveralRulesSendSeveralEvents(t *testing.T) {
+	f := newLocalHoldoutClientFixture(decision.FeatureDecision{
+		Source: decision.Rollout,
+		HoldoutDecisions: []decision.FeatureDecision{
+			makeLocalHoldoutDecision("local_holdout"),
+			makeLocalHoldoutDecision("local_holdout"),
+		},
+	})
+	userContext := newOptimizelyUserContext(f.client, "test_user_1", nil, nil, nil)
+
+	d := f.client.decide(&userContext, f.feature.Key, &decide.Options{})
+
+	assert.Equal(t, "", d.VariationKey)
+	assert.False(t, d.Enabled)
+	// MockProjectConfig has sendFlagDecisions off: holdout events still go out, no rollout event does.
+	assert.Len(t, processedImpressions(f.eventProcessor, decision.Holdout), 2)
+	assert.Len(t, f.eventProcessor.Calls, 2)
+}
+
+func TestDecideLocalHoldoutOnDeliveryRuleSendsEventWithSendFlagDecisionsOff(t *testing.T) {
+	servedVariation := makeTestVariation("on", true)
+	f := newLocalHoldoutClientFixture(decision.FeatureDecision{
+		Experiment:       makeTestExperimentWithVariations("rollout_2", []entities.Variation{servedVariation}),
+		Variation:        &servedVariation,
+		Source:           decision.Rollout,
+		HoldoutDecisions: []decision.FeatureDecision{makeLocalHoldoutDecision("delivery_holdout")},
+	})
+	userContext := newOptimizelyUserContext(f.client, "test_user_1", nil, nil, nil)
+
+	d := f.client.decide(&userContext, f.feature.Key, &decide.Options{})
+
+	assert.Equal(t, "rollout_2", d.RuleKey)
+	assert.Len(t, processedImpressions(f.eventProcessor, decision.Holdout), 1)
+	assert.Len(t, f.eventProcessor.Calls, 1)
+}
+
+func TestDecideDisableDecisionEventSuppressesLocalHoldoutEvents(t *testing.T) {
+	f := newLocalHoldoutClientFixture(decision.FeatureDecision{
+		Source:           decision.Rollout,
+		HoldoutDecisions: []decision.FeatureDecision{makeLocalHoldoutDecision("local_holdout")},
+	})
+	userContext := newOptimizelyUserContext(f.client, "test_user_1", nil, nil, nil)
+
+	f.client.decide(&userContext, f.feature.Key, &decide.Options{DisableDecisionEvent: true})
+
+	f.eventProcessor.AssertNotCalled(t, "ProcessEvent", mock.Anything)
+}
+
+func TestDecideDecisionErrorSuppressesLocalHoldoutEvents(t *testing.T) {
+	holdoutDecision := decision.FeatureDecision{
+		Source:           decision.FeatureTest,
+		HoldoutDecisions: []decision.FeatureDecision{makeLocalHoldoutDecision("local_holdout")},
+	}
+	f := newLocalHoldoutClientFixture(holdoutDecision)
+	cmabErrorMessage := "Failed to fetch CMAB data for experiment exp_1."
+	mockDecisionService := f.client.DecisionService.(*MockDecisionService)
+	mockDecisionService.ExpectedCalls = nil
+	mockDecisionService.On("GetFeatureDecision", mock.Anything, mock.Anything, mock.Anything).Return(holdoutDecision, decide.NewDecisionReasons(nil), errors.New(cmabErrorMessage))
+	userContext := newOptimizelyUserContext(f.client, "test_user_1", nil, nil, nil)
+
+	d := f.client.decide(&userContext, f.feature.Key, &decide.Options{})
+
+	assert.Equal(t, "", d.VariationKey)
+	assert.Equal(t, "", d.RuleKey)
+	assert.False(t, d.Enabled)
+	assert.Contains(t, d.Reasons, cmabErrorMessage)
+	f.eventProcessor.AssertNotCalled(t, "ProcessEvent", mock.Anything)
+}
+
+func makeExcludeTDBypassDecision(localHits ...decision.FeatureDecision) decision.FeatureDecision {
+	servedVariation := makeTestVariation("on", true)
+	globalHit := makeLocalHoldoutDecision("global_holdout")
+	return decision.FeatureDecision{
+		Experiment:        makeTestExperimentWithVariations("rollout_1", []entities.Variation{servedVariation}),
+		Variation:         &servedVariation,
+		Source:            decision.Rollout,
+		HoldoutExperiment: &globalHit.Experiment,
+		HoldoutVariation:  globalHit.Variation,
+		HoldoutDecisions:  append([]decision.FeatureDecision{globalHit}, localHits...),
+	}
+}
+
+func TestLegacyAPIsSendNoHoldoutImpression(t *testing.T) {
+	servedVariation := makeTestVariation("on", true)
+	localHit := decision.FeatureDecision{
+		Experiment:       makeTestExperimentWithVariations("exp_2", []entities.Variation{servedVariation}),
+		Variation:        &servedVariation,
+		Source:           decision.FeatureTest,
+		HoldoutDecisions: []decision.FeatureDecision{makeLocalHoldoutDecision("local_holdout")},
+	}
+	decisions := map[string]decision.FeatureDecision{
+		"local holdout hit":         localHit,
+		"global excludeTD bypassed": makeExcludeTDBypassDecision(),
+	}
+	userContext := entities.UserContext{ID: "test_user_1"}
+
+	for name, featureDecision := range decisions {
+		t.Run(name+"/IsFeatureEnabled", func(t *testing.T) {
+			f := newLocalHoldoutClientFixture(featureDecision)
+			enabled, err := f.client.IsFeatureEnabled(f.feature.Key, userContext)
+			assert.NoError(t, err)
+			assert.True(t, enabled)
+			assert.Empty(t, processedImpressions(f.eventProcessor, decision.Holdout))
+		})
+		t.Run(name+"/GetFeatureVariable", func(t *testing.T) {
+			f := newLocalHoldoutClientFixture(featureDecision)
+			f.client.notificationCenter = notification.NewNotificationCenter()
+			f.config.On("GetVariableByKey", f.feature.Key, "var_1").Return(entities.Variable{Key: "var_1", DefaultValue: "x", Type: entities.String}, nil)
+			_, _, err := f.client.GetFeatureVariable(f.feature.Key, "var_1", userContext)
+			assert.NoError(t, err)
+			assert.Empty(t, processedImpressions(f.eventProcessor, decision.Holdout))
+		})
+		t.Run(name+"/GetDetailedFeatureDecisionUnsafe", func(t *testing.T) {
+			f := newLocalHoldoutClientFixture(featureDecision)
+			_, err := f.client.GetDetailedFeatureDecisionUnsafe(f.feature.Key, userContext, false)
+			assert.NoError(t, err)
+			assert.Empty(t, processedImpressions(f.eventProcessor, decision.Holdout))
+		})
+	}
+}
+
+func TestDecideSendsGlobalExcludeTDHoldoutFirstThenLocalHits(t *testing.T) {
+	f := newLocalHoldoutClientFixture(makeExcludeTDBypassDecision(
+		makeLocalHoldoutDecision("experiment_holdout"),
+		makeLocalHoldoutDecision("delivery_holdout"),
+	))
+	userContext := newOptimizelyUserContext(f.client, "test_user_1", nil, nil, nil)
+
+	d := f.client.decide(&userContext, f.feature.Key, &decide.Options{})
+
+	assert.Equal(t, "rollout_1", d.RuleKey)
+	holdoutEvents := processedImpressions(f.eventProcessor, decision.Holdout)
+	var ruleKeys []string
+	for _, e := range holdoutEvents {
+		ruleKeys = append(ruleKeys, e.Impression.Metadata.RuleKey)
+	}
+	assert.Equal(t, []string{"global_holdout", "experiment_holdout", "delivery_holdout"}, ruleKeys)
+	assert.Len(t, f.eventProcessor.Calls, 3)
+}

pkg/decision/composite_feature_service.go

@@ -66,6 +66,7 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont
 	}
 
 	var featureDecision FeatureDecision
+	var holdoutDecisions []FeatureDecision
 	var err error
 	for _, featureDecisionService := range f.featureServices {
 		var decisionReasons decide.DecisionReasons
@@ -77,24 +78,34 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont
 			return FeatureDecision{}, reasons, err
 		}
 
+		holdoutDecisions = append(holdoutDecisions, featureDecision.HoldoutDecisions...)
+		featureDecision.HoldoutDecisions = holdoutDecisions
 		if featureDecision.Variation != nil {
-			return featureDecision, reasons, err
+			return withFirstHoldout(featureDecision), reasons, err
 		}
 	}
-	return featureDecision, reasons, err
+	return withFirstHoldout(featureDecision), reasons, err
+}
+
+// withFirstHoldout populates the deprecated HoldoutExperiment/HoldoutVariation fields from the first holdout hit.
+func withFirstHoldout(featureDecision FeatureDecision) FeatureDecision {
+	if len(featureDecision.HoldoutDecisions) > 0 {
+		first := featureDecision.HoldoutDecisions[0]
+		featureDecision.HoldoutExperiment = &first.Experiment
+		featureDecision.HoldoutVariation = first.Variation
+	}
+	return featureDecision
 }
 
 // getDecisionWithExcludedTD handles the exclude_targeted_deliveries holdout logic.
 // When a holdout has ExcludeTargetedDeliveries set, AB/MAB/CMAB experiments are
 // blocked (holdout returned) but targeted delivery rules are allowed through.
 func (f CompositeFeatureService) getDecisionWithExcludedTD(holdoutDecision FeatureDecision, decisionContext FeatureDecisionContext, userContext entities.UserContext, options *decide.Options, reasons decide.DecisionReasons) (FeatureDecision, decide.DecisionReasons, error) {
-	holdoutExp := holdoutDecision.Experiment
-	holdoutVar := holdoutDecision.Variation
-
 	reasons.AddInfo("Holdout \"%s\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.", holdoutDecision.Holdout.Key)
 
 	// Skip experiment evaluation entirely (A/B/MAB/CMAB are blocked by holdout).
 	// Evaluate rollout service only (targeted deliveries are excluded from holdout blocking).
+	holdoutDecisions := []FeatureDecision{holdoutDecision}
 	if f.rolloutService != nil {
 		rolloutDecision, rolloutReasons, err := f.rolloutService.GetDecision(decisionContext, userContext, options)
 		reasons.Append(rolloutReasons)
@@ -103,20 +114,19 @@ func (f CompositeFeatureService) getDecisionWithExcludedTD(holdoutDecision Featu
 			reasons.AddError(err.Error())
 			return FeatureDecision{}, reasons, err
 		}
+		holdoutDecisions = append(holdoutDecisions, rolloutDecision.HoldoutDecisions...)
 		if rolloutDecision.Variation != nil {
-			rolloutDecision.HoldoutExperiment = &holdoutExp
-			rolloutDecision.HoldoutVariation = holdoutVar
-			return rolloutDecision, reasons, nil
+			rolloutDecision.HoldoutDecisions = holdoutDecisions
+			return withFirstHoldout(rolloutDecision), reasons, nil
 		}
 	}
 
 	emptyDecision := FeatureDecision{
 		// Match the rollout service's no-match behavior (see RolloutService.GetDecision),
 		// so a served impression under sendFlagDecisions carries ruleType "rollout" rather
 		// than a blank value.
-		Source:            Rollout,
-		HoldoutExperiment: &holdoutExp,
-		HoldoutVariation:  holdoutVar,
+		Source:           Rollout,
+		HoldoutDecisions: holdoutDecisions,
 	}
-	return emptyDecision, reasons, nil
+	return withFirstHoldout(emptyDecision), reasons, nil
 }