fixing comments

YarivHashaiComet · YarivHashaiComet · commit bb34eedd31e7 · 2025-07-17T16:06:08.000+03:00
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/evaluators/AutomationRuleEvaluator.java b/apps/opik-backend/src/main/java/com/comet/opik/api/evaluators/AutomationRuleEvaluator.java
@@ -62,7 +62,8 @@ public abstract sealed class AutomationRuleEvaluator<T> implements AutomationRul
     private final float samplingRate;
 
     @JsonView({View.Public.class, View.Write.class})
-    private final boolean enabled;
+    @Builder.Default
+    private final boolean enabled = true;
 
     @JsonIgnore
     @NotNull private final T code;
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/evaluators/AutomationRuleEvaluatorUpdate.java b/apps/opik-backend/src/main/java/com/comet/opik/api/evaluators/AutomationRuleEvaluatorUpdate.java
@@ -11,6 +11,7 @@
 import jakarta.validation.constraints.NotBlank;
 import jakarta.validation.constraints.NotNull;
 import lombok.AllArgsConstructor;
+import lombok.Builder;
 import lombok.Data;
 import lombok.experimental.SuperBuilder;
 
@@ -43,7 +44,8 @@ public abstract sealed class AutomationRuleEvaluatorUpdate<T> implements Automat
 
     private final float samplingRate;
 
-    private final boolean enabled;
+    @Builder.Default
+    private final boolean enabled = true;
 
     @JsonIgnore
     @NotNull private final T code;
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/OnlineScoringSampler.java b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/OnlineScoringSampler.java
@@ -13,6 +13,7 @@
 import com.comet.opik.domain.evaluators.UserLog;
 import com.comet.opik.infrastructure.OnlineScoringConfig;
 import com.comet.opik.infrastructure.ServiceTogglesConfig;
+import com.comet.opik.infrastructure.log.LogContextAware;
 import com.comet.opik.infrastructure.log.UserFacingLoggingFactory;
 import com.google.common.eventbus.Subscribe;
 import jakarta.inject.Inject;
@@ -121,12 +122,7 @@ private boolean shouldSampleTrace(AutomationRuleEvaluator<?> evaluator, String w
         // Check if rule is enabled first
         if (!evaluator.isEnabled()) {
             // Important to set the workspaceId for logging purposes
-            try (var logContext = wrapWithMdc(Map.of(
-                    UserLog.MARKER, UserLog.AUTOMATION_RULE_EVALUATOR.name(),
-                    "workspace_id", workspaceId,
-                    "rule_id", evaluator.getId().toString(),
-                    "trace_id", trace.id().toString()))) {
-
+            try (var logContext = createTraceLoggingContext(workspaceId, evaluator, trace)) {
                 userFacingLogger.info(
                         "The traceId '{}' was skipped for rule: '{}' as the rule is disabled",
                         trace.id(), evaluator.getName());
@@ -138,12 +134,7 @@ private boolean shouldSampleTrace(AutomationRuleEvaluator<?> evaluator, String w
 
         if (!shouldBeSampled) {
             // Important to set the workspaceId for logging purposes
-            try (var logContext = wrapWithMdc(Map.of(
-                    UserLog.MARKER, UserLog.AUTOMATION_RULE_EVALUATOR.name(),
-                    "workspace_id", workspaceId,
-                    "rule_id", evaluator.getId().toString(),
-                    "trace_id", trace.id().toString()))) {
-
+            try (var logContext = createTraceLoggingContext(workspaceId, evaluator, trace)) {
                 userFacingLogger.info(
                         "The traceId '{}' was skipped for rule: '{}' and per the sampling rate '{}'",
                         trace.id(), evaluator.getName(), evaluator.getSamplingRate());
@@ -187,4 +178,13 @@ private void logSampledTrace(TracesCreated tracesBatch, AutomationRuleEvaluator<
                 tracesBatch.traces().size(),
                 evaluator.getSamplingRate());
     }
+
+    private LogContextAware.Closable createTraceLoggingContext(String workspaceId, AutomationRuleEvaluator<?> evaluator,
+            Trace trace) {
+        return wrapWithMdc(Map.of(
+                UserLog.MARKER, UserLog.AUTOMATION_RULE_EVALUATOR.name(),
+                "workspace_id", workspaceId,
+                "rule_id", evaluator.getId().toString(),
+                "trace_id", trace.id().toString()));
+    }
 }
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/TraceThreadOnlineScoringSamplerListener.java b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/TraceThreadOnlineScoringSamplerListener.java
@@ -9,6 +9,7 @@
 import com.comet.opik.domain.threads.TraceThreadModel;
 import com.comet.opik.domain.threads.TraceThreadService;
 import com.comet.opik.infrastructure.auth.RequestContext;
+import com.comet.opik.infrastructure.log.LogContextAware;
 import com.comet.opik.infrastructure.log.UserFacingLoggingFactory;
 import com.google.common.eventbus.Subscribe;
 import jakarta.inject.Inject;
@@ -136,25 +137,17 @@ private List<TraceThreadSampling> sampleTraceThreads(Map<UUID, TraceThreadModel>
 
                                 // Check if rule is enabled first
                                 if (!evaluator.isEnabled()) {
-                                    try (var logContext = wrapWithMdc(Map.of(
-                                            UserLog.MARKER, UserLog.AUTOMATION_RULE_EVALUATOR.name(),
-                                            UserLog.WORKSPACE_ID, workspaceId,
-                                            UserLog.RULE_ID, evaluator.getId().toString(),
-                                            UserLog.THREAD_MODEL_ID, traceThreadModelId.toString()))) {
-
+                                    try (var logContext = createThreadLoggingContext(workspaceId, evaluator,
+                                            traceThreadModelId)) {
                                         userFacingLogger.info(
                                                 "The threadModelId '{}' was skipped for rule: '{}' as the rule is disabled",
                                                 traceThreadModelId, evaluator.getName());
                                     }
                                 } else {
                                     shouldBeSampled = secureRandom.nextDouble() < evaluator.getSamplingRate();
 
-                                    try (var logContext = wrapWithMdc(Map.of(
-                                            UserLog.MARKER, UserLog.AUTOMATION_RULE_EVALUATOR.name(),
-                                            UserLog.WORKSPACE_ID, workspaceId,
-                                            UserLog.RULE_ID, evaluator.getId().toString(),
-                                            UserLog.THREAD_MODEL_ID, traceThreadModelId.toString()))) {
-
+                                    try (var logContext = createThreadLoggingContext(workspaceId, evaluator,
+                                            traceThreadModelId)) {
                                         if (!shouldBeSampled) {
                                             userFacingLogger.info(
                                                     "The threadModelId '{}' was skipped for rule: '{}' and per the sampling rate '{}'",
@@ -188,4 +181,13 @@ private Map<UUID, Boolean> groupRuleSampling(Map<UUID, Boolean> acc, Map<UUID, B
         acc.putAll(current);
         return acc;
     }
+
+    private LogContextAware.Closable createThreadLoggingContext(String workspaceId,
+            AutomationRuleEvaluator<?> evaluator, UUID traceThreadModelId) {
+        return wrapWithMdc(Map.of(
+                UserLog.MARKER, UserLog.AUTOMATION_RULE_EVALUATOR.name(),
+                UserLog.WORKSPACE_ID, workspaceId,
+                UserLog.RULE_ID, evaluator.getId().toString(),
+                UserLog.THREAD_MODEL_ID, traceThreadModelId.toString()));
+    }
 }
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleModel.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleModel.java
@@ -12,7 +12,7 @@ public sealed interface AutomationRuleModel
     String name();
 
     Float samplingRate();
-    Boolean enabled();
+    boolean enabled();
 
     String createdBy();
     String lastUpdatedBy();
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/LlmAsJudgeAutomationRuleEvaluatorModel.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/LlmAsJudgeAutomationRuleEvaluatorModel.java
@@ -18,7 +18,7 @@ public record LlmAsJudgeAutomationRuleEvaluatorModel(
         String projectName,
         String name,
         Float samplingRate,
-        Boolean enabled,
+        boolean enabled,
         @Json LlmAsJudgeCode code,
         Instant createdAt,
         String createdBy,
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/TraceThreadLlmAsJudgeAutomationRuleEvaluatorModel.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/TraceThreadLlmAsJudgeAutomationRuleEvaluatorModel.java
@@ -17,7 +17,7 @@ public record TraceThreadLlmAsJudgeAutomationRuleEvaluatorModel(
         String projectName,
         String name,
         Float samplingRate,
-        Boolean enabled,
+        boolean enabled,
         @Json TraceThreadLlmAsJudgeCode code,
         Instant createdAt,
         String createdBy,
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/TraceThreadUserDefinedMetricPythonAutomationRuleEvaluatorModel.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/TraceThreadUserDefinedMetricPythonAutomationRuleEvaluatorModel.java
@@ -16,7 +16,7 @@ public record TraceThreadUserDefinedMetricPythonAutomationRuleEvaluatorModel(
         String projectName,
         String name,
         Float samplingRate,
-        Boolean enabled,
+        boolean enabled,
         @Json TraceThreadUserDefinedMetricPythonCode code,
         Instant createdAt,
         String createdBy,
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/UserDefinedMetricPythonAutomationRuleEvaluatorModel.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/UserDefinedMetricPythonAutomationRuleEvaluatorModel.java
@@ -17,7 +17,7 @@ public record UserDefinedMetricPythonAutomationRuleEvaluatorModel(
         String projectName,
         String name,
         Float samplingRate,
-        Boolean enabled,
+        boolean enabled,
         @Json UserDefinedMetricPythonCode code,
         Instant createdAt,
         String createdBy,
diff --git a/apps/opik-backend/src/main/resources/liquibase/db-app-state/migrations/000022_add_enabled_to_automation_rules.sql b/apps/opik-backend/src/main/resources/liquibase/db-app-state/migrations/000022_add_enabled_to_automation_rules.sql
diff --git a/apps/opik-backend/src/main/resources/liquibase/db-app-state/migrations/000023_add_enabled_to_automation_rules.sql b/apps/opik-backend/src/main/resources/liquibase/db-app-state/migrations/000023_add_enabled_to_automation_rules.sql
@@ -0,0 +1,6 @@
+--liquibase formatted sql
+--changeset yariv:000022_add_enabled_to_automation_rules
+
+ALTER TABLE automation_rules ADD COLUMN enabled BOOLEAN NOT NULL DEFAULT TRUE;
+
+--rollback ALTER TABLE automation_rules DROP COLUMN enabled;
diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResourceTest.java
@@ -837,64 +837,42 @@ void update(
             }
         }
 
-        @Test
-        void updateEnabledToDisabled() {
-            // Create an enabled rule
+        @ParameterizedTest
+        @MethodSource
+        @DisplayName("Should update enabled status correctly")
+        void updateEnabledStatus(boolean initialEnabled, boolean targetEnabled, String scenarioDescription) {
+            // Create a rule with initial enabled state
             var automationRuleEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
                     .toBuilder()
-                    .enabled(true)
+                    .enabled(initialEnabled)
                     .build();
             var id = evaluatorsResourceClient.createEvaluator(automationRuleEvaluator, WORKSPACE_NAME, API_KEY);
 
-            // Update to disable it
+            // Update to target enabled state
             var updatedEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorUpdateLlmAsJudge.class)
                     .toBuilder()
                     .projectId(automationRuleEvaluator.getProjectId())
-                    .enabled(false)
+                    .enabled(targetEnabled)
                     .build();
 
             try (var actualResponse = evaluatorsResourceClient.updateEvaluator(
                     id, WORKSPACE_NAME, updatedEvaluator, API_KEY, HttpStatus.SC_NO_CONTENT)) {
                 assertThat(actualResponse.hasEntity()).isFalse();
             }
 
-            // Verify the rule is now disabled
+            // Verify the rule has the expected enabled state
             try (var actualResponse = evaluatorsResourceClient.getEvaluator(
                     id, null, WORKSPACE_NAME, API_KEY, HttpStatus.SC_OK)) {
                 var actualAutomationRuleEvaluator = actualResponse.readEntity(AutomationRuleEvaluator.class);
-                assertThat(actualAutomationRuleEvaluator.isEnabled()).isFalse();
+                assertThat(actualAutomationRuleEvaluator.isEnabled()).isEqualTo(targetEnabled);
                 assertThat(actualAutomationRuleEvaluator.getName()).isEqualTo(updatedEvaluator.getName());
             }
         }
 
-        @Test
-        void updateDisabledToEnabled() {
-            // Create a disabled rule
-            var automationRuleEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
-                    .toBuilder()
-                    .enabled(false)
-                    .build();
-            var id = evaluatorsResourceClient.createEvaluator(automationRuleEvaluator, WORKSPACE_NAME, API_KEY);
-
-            // Update to enable it
-            var updatedEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorUpdateLlmAsJudge.class)
-                    .toBuilder()
-                    .projectId(automationRuleEvaluator.getProjectId())
-                    .enabled(true)
-                    .build();
-
-            try (var actualResponse = evaluatorsResourceClient.updateEvaluator(
-                    id, WORKSPACE_NAME, updatedEvaluator, API_KEY, HttpStatus.SC_NO_CONTENT)) {
-                assertThat(actualResponse.hasEntity()).isFalse();
-            }
-
-            // Verify the rule is now enabled
-            try (var actualResponse = evaluatorsResourceClient.getEvaluator(
-                    id, null, WORKSPACE_NAME, API_KEY, HttpStatus.SC_OK)) {
-                var actualAutomationRuleEvaluator = actualResponse.readEntity(AutomationRuleEvaluator.class);
-                assertThat(actualAutomationRuleEvaluator.isEnabled()).isTrue();
-                assertThat(actualAutomationRuleEvaluator.getName()).isEqualTo(updatedEvaluator.getName());
-            }
+        static Stream<Arguments> updateEnabledStatus() {
+            return Stream.of(
+                    Arguments.of(true, false, "enabled to disabled"),
+                    Arguments.of(false, true, "disabled to enabled"));
         }
     }
 
diff --git a/apps/opik-frontend/src/components/pages/OnlineEvaluationPage/OnlineEvaluationPage.tsx b/apps/opik-frontend/src/components/pages/OnlineEvaluationPage/OnlineEvaluationPage.tsx
@@ -100,7 +100,6 @@ const DEFAULT_COLUMNS: ColumnData<EvaluatorsRule>[] = [
     id: "enabled",
     label: "Enabled",
     type: COLUMN_TYPE.boolean,
-    accessorFn: (row) => (row.enabled ?? true ? "enabled" : "disabled"),
     cell: RuleEnabledCell as never,
   },
 ];
diff --git a/apps/opik-frontend/src/components/pages/TracesPage/RulesTab/RulesTab.tsx b/apps/opik-frontend/src/components/pages/TracesPage/RulesTab/RulesTab.tsx
@@ -90,7 +90,6 @@ const DEFAULT_COLUMNS: ColumnData<EvaluatorsRule>[] = [
     id: "enabled",
     label: "Enabled",
     type: COLUMN_TYPE.boolean,
-    accessorFn: (row) => (row.enabled ?? true ? "enabled" : "disabled"),
     cell: RuleEnabledCell as never,
   },
 ];
diff --git a/apps/opik-frontend/src/constants/filters.ts b/apps/opik-frontend/src/constants/filters.ts
@@ -10,10 +10,11 @@ export const DEFAULT_OPERATORS: DropdownOption<FilterOperator>[] = [
   { label: "contains", value: "contains" },
 ];
 
-export const DEFAULT_OPERATOR_MAP: Record<COLUMN_TYPE, FilterOperator> = {
+export const DEFAULT_OPERATOR_MAP: Partial<
+  Record<COLUMN_TYPE, FilterOperator>
+> = {
   [COLUMN_TYPE.string]: "contains",
   [COLUMN_TYPE.number]: ">=",
-  [COLUMN_TYPE.boolean]: "=",
   [COLUMN_TYPE.list]: "contains",
   [COLUMN_TYPE.time]: ">=",
   [COLUMN_TYPE.dictionary]: "=",
@@ -24,9 +25,8 @@ export const DEFAULT_OPERATOR_MAP: Record<COLUMN_TYPE, FilterOperator> = {
   [COLUMN_TYPE.errors]: "is_not_empty",
 };
 
-export const OPERATORS_MAP: Record<
-  COLUMN_TYPE,
-  DropdownOption<FilterOperator>[]
+export const OPERATORS_MAP: Partial<
+  Record<COLUMN_TYPE, DropdownOption<FilterOperator>[]>
 > = {
   [COLUMN_TYPE.string]: [
     {
@@ -72,16 +72,7 @@ export const OPERATORS_MAP: Record<
       value: "<=",
     },
   ],
-  [COLUMN_TYPE.boolean]: [
-    {
-      label: "=",
-      value: "=",
-    },
-    {
-      label: "≠",
-      value: "!=",
-    },
-  ],
+
   [COLUMN_TYPE.cost]: [
     {
       label: "=",
