[postprocessor] fix mutable data merge logic (#1661)

thirdeye-plugins/thirdeye-postprocessors/src/main/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessor.java

@@ -504,10 +504,23 @@ protected List<AnomalyDTO> doMerge(final List<AnomalyDTO> operatorAnomalies,
             // if an anomaly is in ignore state and has never been notified, but is now changing to not ignore, then ensure it is notified 
             // by hack on the createTime to ensure the notification logic finds this anomaly
             // see spec decision table https://docs.google.com/document/d/1bSbv4XhTQsdGR1XVM_dYL1cK9Q6JlntvzNYmmMiXQRI/edit
-            // FIXME CYRIL - something similar should also be done at the parent level 
             if (isIgnore(previousAnomaly) && !previousAnomaly.isNotified() && !isIgnore(anomaly)) {
               previousAnomaly.setCreateTime(new Timestamp(System.currentTimeMillis()));
             }
+            if (isIgnore(previousAnomaly) != isIgnore(anomaly)) {
+              // need to update the parentCandidate/ignoredParentCandidate if necessary
+              if (previousAnomaly == ignoredParentCandidate) {
+                // the current ignoredParentCandidate is not ignored now - move it to parentCandidate
+                ignoredParentCandidate = null;
+                optional(parentCandidate).ifPresent(anomaliesToUpdate::add);
+                parentCandidate = previousAnomaly;
+              } else if (previousAnomaly == parentCandidate) {
+                // the current parentCandidate is ignored now - move it to ignoredParentCandidate 
+                parentCandidate = null;
+                optional(ignoredParentCandidate).ifPresent(anomaliesToUpdate::add);
+                ignoredParentCandidate = previousAnomaly;
+              }
+            }
             // update the existing anomaly with minor changes - drop the new anomaly
             updateAnomalyWithNewValues(previousAnomaly, anomaly);
             // labels depend on the values - so pick the latest labels

thirdeye-plugins/thirdeye-postprocessors/src/test/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessorTest.java

@@ -18,6 +18,7 @@
 import static ai.startree.thirdeye.plugins.postprocessor.AnomalyMergerPostProcessor.newAfterReplayLabel;
 import static ai.startree.thirdeye.plugins.postprocessor.AnomalyMergerPostProcessor.newOutdatedLabel;
 import static ai.startree.thirdeye.spi.Constants.VANILLA_OBJECT_MAPPER;
+import static ai.startree.thirdeye.spi.util.AnomalyUtils.isIgnore;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
@@ -732,6 +733,74 @@ public void testReplayRule3BisUC2(final String name,
       assertThat(e2.getCreateTime().getTime()).isEqualTo(0);
     }
   }
+
+  @Test
+  public void testReplayRule3bisUC1IgnoredToNotIgnoredFollowedByAnomalyInMergeFrame() {
+    // ensure the Rule 3bis UC1 implementation does not break merging behavior for anomalies that follow
+    // existing: (existing parentWithNoChild: ignored, not notified), existing parentWithNoChild ignored)
+    // at replay (replay (not ignored), replay ignored)
+    // input of doMerge (existing parentWithNoChild: ignored, not notified), replay (not ignored), existing parentWithNoChild ignored, replay ignored)
+    final AnomalyDTO e1 = existingAnomaly(JANUARY_1_2021_01H, JANUARY_1_2021_02H)
+        .setAnomalyLabels(listOf(new AnomalyLabelDTO().setName("NEW_LABEL").setIgnore(true)));
+    final AnomalyDTO e2 = existingAnomaly(JANUARY_1_2021_02H, JANUARY_1_2021_03H)
+        .setAnomalyLabels(listOf(new AnomalyLabelDTO().setName("SOME_LABEL")
+            .setIgnore(true)));
+    existingAnomalies = listOf(e1, e2);
+    // no merge
+    detectionSpec.setMergeMaxGap("PT0S");
+    detectionSpec.setReNotifyAbsoluteThreshold(-1.);
+    detectionSpec.setReNotifyPercentageThreshold(-1.);
+    detectionMerger = new AnomalyMergerPostProcessor(detectionSpec);
+
+    // new anomaly at 1H is not ignored anymore
+    final AnomalyDTO r1 = newAnomaly(JANUARY_1_2021_01H, JANUARY_1_2021_02H);
+    // anomaly at 2H does not change
+    final AnomalyDTO r2 = newAnomaly(JANUARY_1_2021_02H, JANUARY_1_2021_03H)
+        .setAnomalyLabels(listOf(new AnomalyLabelDTO().setName("SOME_LABEL")
+            .setIgnore(true)));
+    final Interval detectionInterval = new Interval(JANUARY_1_2021_01H, JANUARY_1_2021_03H, UTC);
+    final Set<AnomalyDTO> output = detectionMerger.merge(listOf(r1, r2), detectionInterval);
+
+    assertThat(output).isEqualTo(Set.of(e1, e2));
+    assertThat(e1.getAnomalyLabels()).isNull();
+    assertThat(isIgnore(e1)).isFalse();
+    assertThat(e2.getAnomalyLabels().size()).isEqualTo(1);
+    assertThat(isIgnore(e2)).isTrue();
+    // e1 create time was changed to trigger notification
+    assertThat(e1.getCreateTime().getTime()).isGreaterThan(0);
+    // e2 create time is not changed ()
+    assertThat(e1.getCreateTime().getTime()).isGreaterThan(0);
+  }
+
+  @Test
+  public void testReplayRule3bisUC1NotIgnoredToIgnoredFollowedByAnomalyInMergeFrame() {
+    // ensure the Rule 3bis UC1 implementation does not break merging behavior for anomalies that follow
+    // existing: (existing parentWithNoChild: not ignored), existing parentWithNoChild not ignored)
+    // at replay (replay (now ignored), replay not ignored)
+    // input of doMerge (existing parentWithNoChild: not ignored), replay (now ignored), existing parentWithNoChild not ignored, replay not ignored)
+    final AnomalyDTO e1 = existingAnomaly(JANUARY_1_2021_01H, JANUARY_1_2021_02H);
+    final AnomalyDTO e2 = existingAnomaly(JANUARY_1_2021_02H, JANUARY_1_2021_03H);
+    existingAnomalies = listOf(e1, e2);
+    // no merge
+    detectionSpec.setMergeMaxGap("PT0S");
+    detectionSpec.setReNotifyAbsoluteThreshold(-1.);
+    detectionSpec.setReNotifyPercentageThreshold(-1.);
+    detectionMerger = new AnomalyMergerPostProcessor(detectionSpec);
+
+    // new anomaly at 1H is now ignored
+    final AnomalyDTO r1 = newAnomaly(JANUARY_1_2021_01H, JANUARY_1_2021_02H)
+        .setAnomalyLabels(listOf(new AnomalyLabelDTO().setName("SOME_LABEL")
+            .setIgnore(true)));
+    // anomaly at 2H does not change
+    final AnomalyDTO r2 = newAnomaly(JANUARY_1_2021_02H, JANUARY_1_2021_03H);
+    final Interval detectionInterval = new Interval(JANUARY_1_2021_01H, JANUARY_1_2021_03H, UTC);
+    final Set<AnomalyDTO> output = detectionMerger.merge(listOf(r1, r2), detectionInterval);
+
+    assertThat(output).isEqualTo(Set.of(e1, e2));
+    assertThat(e1.getAnomalyLabels().size()).isEqualTo(1);
+    assertThat(isIgnore(e1)).isTrue();
+    assertThat(e2.getAnomalyLabels()).isNull();
+  }
 
   @Test(dataProvider = "rule3bisSameAsRule3Cases")
   public void testReplayRule3BisBehaveLikeRule3WithChildInsideParentUpdated(