Do not override non-bandit assignments when getBanditAction is called with no actions (#115)

* Only perform no-action-check when bandit is assigned; adjust test case

* return variation if assigned

* bump version

* cleanup commented out line

---------

Co-authored-by: Aaron Silverman <[email protected]>

Author: giorgiomartini0

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@eppo/js-client-sdk-common",
-  "version": "4.0.0",
+  "version": "4.0.1",
   "description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)",
   "main": "dist/index.js",
   "files": [

src/client/eppo-client-with-bandits.spec.ts

@@ -233,7 +233,7 @@ describe('EppoClient Bandits E2E test', () => {
       expect(banditEvent.action).toBe('adidas');
     });
 
-    it('Does not log if no actions provided', () => {
+    it('Logs assignment but not bandit action if no actions provided', () => {
       const banditAssignment = client.getBanditAction(
         'banner_bandit_flag',
         'eve',
@@ -242,10 +242,10 @@ describe('EppoClient Bandits E2E test', () => {
         'control',
       );
 
-      expect(banditAssignment.variation).toBe('control');
+      expect(banditAssignment.variation).toBe('banner_bandit');
       expect(banditAssignment.action).toBeNull();
 
-      expect(mockLogAssignment).not.toHaveBeenCalled();
+      expect(mockLogAssignment).toHaveBeenCalledTimes(1);
       expect(mockLogBanditAction).not.toHaveBeenCalled();
     });
 
@@ -263,7 +263,7 @@ describe('EppoClient Bandits E2E test', () => {
           });
       });
 
-      it('Returns default value when graceful mode is on', () => {
+      it('Returns null action when graceful mode is on', () => {
         client.setIsGracefulFailureMode(true);
         const banditAssignment = client.getBanditActionDetails(
           flagKey,
@@ -272,7 +272,7 @@ describe('EppoClient Bandits E2E test', () => {
           actions,
           'control',
         );
-        expect(banditAssignment.variation).toBe('control');
+        expect(banditAssignment.variation).toBe('banner_bandit');
         expect(banditAssignment.action).toBeNull();
 
         expect(
@@ -282,26 +282,25 @@ describe('EppoClient Bandits E2E test', () => {
           configFetchedAt: expect.any(String),
           configPublishedAt: '2024-04-17T19:40:53.716Z',
           environmentName: 'Test',
-          flagEvaluationCode: 'ASSIGNMENT_ERROR',
+          flagEvaluationCode: 'BANDIT_ERROR',
           flagEvaluationDescription: 'Error evaluating bandit action: Intentional Error For Test',
-          matchedAllocation: null,
+          matchedAllocation: {
+            allocationEvaluationCode: AllocationEvaluationCode.MATCH,
+            key: 'training',
+            orderPosition: 2,
+          },
           matchedRule: null,
-          unevaluatedAllocations: [
+          unevaluatedAllocations: [],
+          unmatchedAllocations: [
             {
-              allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED,
+              allocationEvaluationCode: AllocationEvaluationCode.TRAFFIC_EXPOSURE_MISS,
               key: 'analysis',
               orderPosition: 1,
             },
-            {
-              allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED,
-              key: 'training',
-              orderPosition: 2,
-            },
           ],
-          unmatchedAllocations: [],
-          variationKey: null,
-          variationValue: null,
-          banditKey: null,
+          variationKey: 'banner_bandit',
+          variationValue: 'banner_bandit',
+          banditKey: 'banner_bandit',
           banditAction: null,
         };
         expect(banditAssignment.evaluationDetails).toEqual(expectedEvaluationDetails);

src/client/eppo-client.ts

@@ -462,99 +462,116 @@ export default class EppoClient {
     actions: BanditActions,
     defaultValue: string,
   ): IAssignmentDetails<string> {
-    const flagEvaluationDetailsBuilder = this.flagEvaluationDetailsBuilder(flagKey);
-    const defaultResult = { variation: defaultValue, action: null };
     let variation = defaultValue;
     let action: string | null = null;
-    try {
-      const banditVariations = this.banditVariationConfigurationStore?.get(flagKey);
-      if (banditVariations && !Object.keys(actions).length) {
-        // No actions passed for a flag known to have an active bandit, so we just return the default values so that
-        // we don't log a variation or bandit assignment
-        return {
-          ...defaultResult,
-          evaluationDetails: flagEvaluationDetailsBuilder.buildForNoneResult(
-            'NO_ACTIONS_SUPPLIED_FOR_BANDIT',
-            'No bandit actions passed for a flag known to have an active bandit',
-          ),
-        };
-      }
 
+    // Initialize with a generic evaluation details. This will mutate as the function progresses.
+    let evaluationDetails: IFlagEvaluationDetails = this.flagEvaluationDetailsBuilder(
+      flagKey,
+    ).buildForNoneResult(
+      'ASSIGNMENT_ERROR',
+      'Unexpected error getting assigned variation for bandit action',
+    );
+    try {
       // Get the assigned variation for the flag with a possible bandit
       // Note for getting assignments, we don't care about context
       const nonContextualSubjectAttributes =
         this.ensureNonContextualSubjectAttributes(subjectAttributes);
-      const { variation: _variation, evaluationDetails } = this.getStringAssignmentDetails(
-        flagKey,
-        subjectKey,
-        nonContextualSubjectAttributes,
-        defaultValue,
-      );
-      variation = _variation;
+      const { variation: assignedVariation, evaluationDetails: assignmentEvaluationDetails } =
+        this.getStringAssignmentDetails(
+          flagKey,
+          subjectKey,
+          nonContextualSubjectAttributes,
+          defaultValue,
+        );
+      variation = assignedVariation;
+      evaluationDetails = assignmentEvaluationDetails;
 
       // Check if the assigned variation is an active bandit
       // Note: the reason for non-bandit assignments include the subject being bucketed into a non-bandit variation or
       // a rollout having been done.
+      const banditVariations = this.banditVariationConfigurationStore?.get(flagKey);
       const banditKey = banditVariations?.find(
         (banditVariation) => banditVariation.variationValue === variation,
       )?.key;
 
       if (banditKey) {
-        // Retrieve the model parameters for the bandit
-        const banditParameters = this.banditModelConfigurationStore?.get(banditKey);
-
-        if (!banditParameters) {
-          throw new Error('No model parameters for bandit ' + banditKey);
-        }
-
-        const banditModelData = banditParameters.modelData;
-        const contextualSubjectAttributes =
-          this.ensureContextualSubjectAttributes(subjectAttributes);
-        const actionsWithContextualAttributes = this.ensureActionsWithContextualAttributes(actions);
-        const banditEvaluation = this.banditEvaluator.evaluateBandit(
+        evaluationDetails.banditKey = banditKey;
+        action = this.evaluateBanditAction(
           flagKey,
           subjectKey,
-          contextualSubjectAttributes,
-          actionsWithContextualAttributes,
-          banditModelData,
+          subjectAttributes,
+          actions,
+          banditKey,
+          evaluationDetails,
         );
-        action = banditEvaluation.actionKey;
         evaluationDetails.banditAction = action;
-        evaluationDetails.banditKey = banditKey;
-
-        const banditEvent: IBanditEvent = {
-          timestamp: new Date().toISOString(),
-          featureFlag: flagKey,
-          bandit: banditKey,
-          subject: subjectKey,
-          action,
-          actionProbability: banditEvaluation.actionWeight,
-          optimalityGap: banditEvaluation.optimalityGap,
-          modelVersion: banditParameters.modelVersion,
-          subjectNumericAttributes: contextualSubjectAttributes.numericAttributes,
-          subjectCategoricalAttributes: contextualSubjectAttributes.categoricalAttributes,
-          actionNumericAttributes: actionsWithContextualAttributes[action].numericAttributes,
-          actionCategoricalAttributes:
-            actionsWithContextualAttributes[action].categoricalAttributes,
-          metaData: this.buildLoggerMetadata(),
-          evaluationDetails,
-        };
-        this.logBanditAction(banditEvent);
       }
-      return { variation, action, evaluationDetails };
     } catch (err) {
-      logger.error('Error evaluating bandit action', err);
+      logger.error('Error determining bandit action', err);
       if (!this.isGracefulFailureMode) {
         throw err;
       }
-      return {
-        ...defaultResult,
-        evaluationDetails: flagEvaluationDetailsBuilder.buildForNoneResult(
-          'ASSIGNMENT_ERROR',
-          `Error evaluating bandit action: ${err.message}`,
-        ),
-      };
+      if (variation) {
+        // If we have a variation, the assignment succeeded and the error was with the bandit part.
+        // Update the flag evaluation code to indicate that
+        evaluationDetails.flagEvaluationCode = 'BANDIT_ERROR';
+      }
+      evaluationDetails.flagEvaluationDescription = `Error evaluating bandit action: ${err.message}`;
+    }
+    return { variation, action, evaluationDetails };
+  }
+
+  private evaluateBanditAction(
+    flagKey: string,
+    subjectKey: string,
+    subjectAttributes: BanditSubjectAttributes,
+    actions: BanditActions,
+    banditKey: string,
+    evaluationDetails: IFlagEvaluationDetails,
+  ): string | null {
+    // If no actions, there is nothing to do
+    if (!Object.keys(actions).length) {
+      return null;
+    }
+    // Retrieve the model parameters for the bandit
+    const banditParameters = this.banditModelConfigurationStore?.get(banditKey);
+
+    if (!banditParameters) {
+      throw new Error('No model parameters for bandit ' + banditKey);
     }
+
+    const banditModelData = banditParameters.modelData;
+    const contextualSubjectAttributes = this.ensureContextualSubjectAttributes(subjectAttributes);
+    const actionsWithContextualAttributes = this.ensureActionsWithContextualAttributes(actions);
+    const banditEvaluation = this.banditEvaluator.evaluateBandit(
+      flagKey,
+      subjectKey,
+      contextualSubjectAttributes,
+      actionsWithContextualAttributes,
+      banditModelData,
+    );
+    const action = banditEvaluation.actionKey;
+
+    const banditEvent: IBanditEvent = {
+      timestamp: new Date().toISOString(),
+      featureFlag: flagKey,
+      bandit: banditKey,
+      subject: subjectKey,
+      action,
+      actionProbability: banditEvaluation.actionWeight,
+      optimalityGap: banditEvaluation.optimalityGap,
+      modelVersion: banditParameters.modelVersion,
+      subjectNumericAttributes: contextualSubjectAttributes.numericAttributes,
+      subjectCategoricalAttributes: contextualSubjectAttributes.categoricalAttributes,
+      actionNumericAttributes: actionsWithContextualAttributes[action].numericAttributes,
+      actionCategoricalAttributes: actionsWithContextualAttributes[action].categoricalAttributes,
+      metaData: this.buildLoggerMetadata(),
+      evaluationDetails,
+    };
+    this.logBanditAction(banditEvent);
+
+    return action;
   }
 
   private ensureNonContextualSubjectAttributes(

src/flag-evaluation-details-builder.ts

@@ -8,6 +8,7 @@ export const flagEvaluationCodes = [
   'ASSIGNMENT_ERROR',
   'DEFAULT_ALLOCATION_NULL',
   'NO_ACTIONS_SUPPLIED_FOR_BANDIT',
+  'BANDIT_ERROR',
 ] as const;
 
 export type FlagEvaluationCode = typeof flagEvaluationCodes[number];