Skip to content

Commit 09071d9

Browse files
Mat001claude
andcommitted
fix: use rule Key instead of ID for local holdout lookup (#454)
GetLocalDecisionForRule was called with experiment.ID, but ruleHoldoutsMap is keyed by rule Keys (from datafile includedRules). The mismatch caused local holdouts to silently never match. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9266ab4 commit 09071d9

3 files changed

Lines changed: 7 additions & 7 deletions

File tree

pkg/decision/feature_experiment_service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (f FeatureExperimentService) GetDecision(decisionContext FeatureDecisionCon
6565
// [FSSDK-12369] Check local holdouts targeting this experiment rule.
6666
// Local holdouts are evaluated after forced decisions but before audience/traffic checks.
6767
if f.holdoutService != nil {
68-
holdoutDecision, holdoutReasons, _ := f.holdoutService.GetLocalDecisionForRule(featureExperiment.ID, decisionContext.ProjectConfig, userContext, options)
68+
holdoutDecision, holdoutReasons, _ := f.holdoutService.GetLocalDecisionForRule(featureExperiment.Key, decisionContext.ProjectConfig, userContext, options)
6969
reasons.Append(holdoutReasons)
7070
if holdoutDecision.Variation != nil {
7171
// User is in a local holdout — return holdout decision, skip rule evaluation

pkg/decision/rollout_service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func (r RolloutService) getLocalHoldoutDecision(exp *entities.Experiment, decisi
196196
if r.holdoutService == nil {
197197
return nil, reasons
198198
}
199-
holdoutDecision, holdoutReasons, _ := r.holdoutService.GetLocalDecisionForRule(exp.ID, decisionContext.ProjectConfig, userContext, options)
199+
holdoutDecision, holdoutReasons, _ := r.holdoutService.GetLocalDecisionForRule(exp.Key, decisionContext.ProjectConfig, userContext, options)
200200
reasons.Append(holdoutReasons)
201201
if holdoutDecision.Variation != nil {
202202
return &holdoutDecision, reasons

pkg/decision/rollout_service_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionLocalHoldout_RegularRule_UserBu
397397
{EntityID: "ho_var_1", EndOfRange: 10000}, // 100% — user always bucketed
398398
},
399399
}
400-
s.mockConfig.On("GetHoldoutsForRule", testExp1112.ID).Return([]entities.Holdout{localHoldout})
400+
s.mockConfig.On("GetHoldoutsForRule", testExp1112.Key).Return([]entities.Holdout{localHoldout})
401401
s.mockLogger.On("Debug", mock.Anything).Return()
402402
s.mockLogger.On("Info", mock.Anything).Return()
403403

@@ -432,7 +432,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionLocalHoldout_RegularRule_UserMi
432432
{EntityID: "ho_var_1", EndOfRange: 0}, // 0% — user never bucketed
433433
},
434434
}
435-
s.mockConfig.On("GetHoldoutsForRule", testExp1112.ID).Return([]entities.Holdout{localHoldout})
435+
s.mockConfig.On("GetHoldoutsForRule", testExp1112.Key).Return([]entities.Holdout{localHoldout})
436436
s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams, mock.Anything).Return(true, true, s.reasons)
437437

438438
expectedVariation := testExp1112Var2222
@@ -489,9 +489,9 @@ func (s *RolloutServiceTestSuite) TestGetDecisionLocalHoldout_FallbackRule_UserB
489489
}
490490

491491
// Regular rules miss the holdout; fallback rule hits it
492-
s.mockConfig.On("GetHoldoutsForRule", testExp1112.ID).Return([]entities.Holdout{holdoutMiss})
493-
s.mockConfig.On("GetHoldoutsForRule", testExp1117.ID).Return([]entities.Holdout{holdoutMiss})
494-
s.mockConfig.On("GetHoldoutsForRule", testExp1118.ID).Return([]entities.Holdout{holdoutHit})
492+
s.mockConfig.On("GetHoldoutsForRule", testExp1112.Key).Return([]entities.Holdout{holdoutMiss})
493+
s.mockConfig.On("GetHoldoutsForRule", testExp1117.Key).Return([]entities.Holdout{holdoutMiss})
494+
s.mockConfig.On("GetHoldoutsForRule", testExp1118.Key).Return([]entities.Holdout{holdoutHit})
495495

496496
// Audience fails for both regular rules so they continue to the next rule
497497
s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams, mock.Anything).Return(false, true, s.reasons)

0 commit comments

Comments
 (0)