feedback from PR

Author: aarsilv

src/cache/abstract-assignment-cache.ts

@@ -2,31 +2,28 @@ import { getMD5Hash } from '../obfuscation';
 
 import { LRUCache } from './lru-cache';
 
-type FlagAssignmentCacheValue = {
-  allocationKey: string;
-  variationKey: string;
-};
-
-type BanditAssignmentCacheValue = {
-  banditKey: string;
-  actionKey: string;
-};
-
-export type AssignmentCacheValue = FlagAssignmentCacheValue | BanditAssignmentCacheValue;
-
+/**
+ * Assignment cache keys are only on the subject and flag level, while the entire value is used
+ * for uniqueness checking. This way that if an assigned variation or bandit action changes for a
+ * flag, it evicts the old one. Then, if an older assignment is later reassigned, it will be treated
+ * as new.
+ */
 export type AssignmentCacheKey = {
   subjectKey: string;
   flagKey: string;
 };
 
+export type AssignmentCacheValue = Record<string, string>;
+
 export type AssignmentCacheEntry = AssignmentCacheKey & AssignmentCacheValue;
 
 /** Converts an {@link AssignmentCacheKey} to a string. */
 export function assignmentCacheKeyToString({ subjectKey, flagKey }: AssignmentCacheKey): string {
   return getMD5Hash([subjectKey, flagKey].join(';'));
 }
 
-export function assignmentCacheValueToString(cacheValue: Record<string, string>): string {
+/** Converts an {@link AssignmentCacheValue} to a string. */
+export function assignmentCacheValueToString(cacheValue: AssignmentCacheValue): string {
   return getMD5Hash(Object.values(cacheValue).join(';'));
 }
 

src/client/eppo-client.ts

@@ -652,16 +652,12 @@ export default class EppoClient {
   }
 
   private logBanditAction(banditEvent: IBanditEvent): void {
+    // First we check if this bandit action has been logged before
     const subjectKey = banditEvent.subject;
     const flagKey = banditEvent.featureFlag;
     const banditKey = banditEvent.bandit;
     const actionKey = banditEvent.action ?? '__eppo_no_action';
 
-    // What our bandit assignment cache cares about for avoiding logging duplicate bandit assignments,
-    // if one is active. Like the flag assignment cache, entries are only stored for a given flag
-    // and subject. However, Bandit and action keys are also used for determining assignment uniqueness.
-    // This means that if a flag's bandit assigns one action, and then later a new action, the first
-    // one will be evicted from the cache. If later assigned again, it will be treated as new.
     const banditAssignmentCacheProperties = {
       flagKey,
       subjectKey,
@@ -674,13 +670,15 @@ export default class EppoClient {
       return;
     }
 
+    // If no logger defined, queue up the events (up to a max) to flush if a logger is later defined
     if (!this.banditLogger) {
       // No bandit logger set; enqueue the event in case a logger is later set
       if (this.queuedBanditEvents.length < MAX_EVENT_QUEUE_SIZE) {
         this.queuedBanditEvents.push(banditEvent);
       }
       return;
     }
+
     // If here, we have a logger and a new assignment to be logged
     try {
       this.banditLogger.logBanditAction(banditEvent);