[autorevert] update failure threshold to 3 for autorevert eligibility (#7224)

Changes:
- Failure side now requires 3+ failure events; if less than that, it
schedules restarts on the first (oldest) failed commit sequentially
(allowing pending events to resolve)
- Success and failed sides now issue restarts independently (should
speed up the whole process a bit)

Author: izaitsevfb

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/signal.py

@@ -50,6 +50,7 @@ class IneligibleReason(Enum):
     NO_SUCCESSES = "no_successes"
     NO_PARTITION = "no_partition"  # insufficient commit history to form partitions
     INFRA_NOT_CONFIRMED = "infra_not_confirmed"  # infra check not confirmed
+    INSUFFICIENT_FAILURES = "insufficient_failures"  # not enough failures to make call
     PENDING_GAP = "pending_gap"  # unknown/pending commits present
 
 
@@ -366,21 +367,29 @@ def process_valid_autorevert_pattern(
                 restart_commits.add(c.head_sha)
 
         infra_check_result = partition.confirm_not_an_infra_issue()
-        # note re: event_count < 2:
+        # note re: event_count < 3:
         # this is a confidence heuristic to detect flakiness, can adjust as needed
         if (
             infra_check_result == InfraCheckResult.RESTART_FAILURE
-            or partition.failure_events_count() < 2
+            or partition.failure_events_count() < 3
         ):
             if not partition.failed[-1].has_pending:
                 # restarting oldest failed
                 restart_commits.add(partition.failed[-1].head_sha)
             else:
-                return Ineligible(
-                    IneligibleReason.INFRA_NOT_CONFIRMED,
-                    f"waiting on pending events on suspected failure side: {partition.failed[-1].head_sha}",
-                )
-        elif (
+                if infra_check_result == InfraCheckResult.RESTART_FAILURE:
+                    return Ineligible(
+                        IneligibleReason.INFRA_NOT_CONFIRMED,
+                        f"waiting on pending events on suspected failure side: {partition.failed[-1].head_sha}",
+                    )
+                else:
+                    return Ineligible(
+                        IneligibleReason.INSUFFICIENT_FAILURES,
+                        f"insufficient failures to make call, "
+                        f"pending events on suspected failure side: {partition.failed[-1].head_sha}",
+                    )
+
+        if (
             infra_check_result == InfraCheckResult.RESTART_SUCCESS
             or partition.success_events_count() < 2
         ):

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/tests/test_signal.py

@@ -171,7 +171,11 @@ def test_confirm_not_an_infra_issue_false_no_bracketing_success(self):
         )
 
     def test_detect_autorevert_pattern_basic(self):
-        # Commits newest -> older
+        # Commits newest -> older. With 3 failures total, we meet the new 3+ threshold
+        c_newest = SignalCommit(
+            head_sha="sha_newest",
+            events=[self._ev("job", SignalStatus.FAILURE, 7)],
+        )
         c_newer = SignalCommit(
             head_sha="sha_newer",
             events=[self._ev("job", SignalStatus.FAILURE, 5)],
@@ -189,16 +193,16 @@ def test_detect_autorevert_pattern_basic(self):
             ],
         )
         s = Signal(
-            key="job", workflow_name="wf", commits=[c_newer, c_suspected, c_base]
+            key="job",
+            workflow_name="wf",
+            commits=[c_newest, c_newer, c_suspected, c_base],
         )
         res = s.process_valid_autorevert_pattern()
         self.assertIsNotNone(res)
         self.assertIsInstance(res, AutorevertPattern)
         self.assertEqual(res.workflow_name, "wf")
         # newer failing commits are those after the suspected one (newest->older)
-        self.assertEqual(
-            res.newer_failing_commits, ["sha_newer"]
-        )  # only newer of the two
+        self.assertEqual(res.newer_failing_commits, ["sha_newest", "sha_newer"])
         self.assertEqual(res.suspected_commit, "sha_mid")
         self.assertEqual(res.older_successful_commit, "sha_old")
 
@@ -245,8 +249,10 @@ def test_insufficient_failures_returns_restart_oldest_failed_when_no_pending(sel
         self.assertTrue(hasattr(res, "commit_shas"))
         self.assertIn("sha_failed", res.commit_shas)
 
-    def test_insufficient_failures_infra_not_confirmed_when_pending_on_failed(self):
-        # Only one failure overall, but the failed commit also has pending → ineligible awaiting confirmation
+    def test_insufficient_failures_returns_insufficient_failures_when_pending_on_failed(
+        self,
+    ):
+        # Only one failure overall, but the failed commit also has pending → ineligible due to insufficient failures
         c_failed_pending = SignalCommit(
             head_sha="sha_failed_pend",
             events=[
@@ -264,13 +270,17 @@ def test_insufficient_failures_infra_not_confirmed_when_pending_on_failed(self):
         s = Signal(key="job", workflow_name="wf", commits=[c_failed_pending, c_base])
         res = s.process_valid_autorevert_pattern()
         self.assertIsInstance(res, Ineligible)
-        self.assertEqual(res.reason, IneligibleReason.INFRA_NOT_CONFIRMED)
+        self.assertEqual(res.reason, IneligibleReason.INSUFFICIENT_FAILURES)
         self.assertIn("sha_failed_pend", res.message)
 
     def test_insufficient_successes_returns_restart_newest_success_when_no_pending(
         self,
     ):
-        # Ensure we have at least two failure events to avoid the failure-side heuristic
+        # Ensure we have at least three failure events to avoid the failure-side heuristic
+        c_fail_newest = SignalCommit(
+            head_sha="sha_fail_newest",
+            events=[self._ev("job", SignalStatus.FAILURE, 11)],
+        )
         c_fail_new = SignalCommit(
             head_sha="sha_fail_new",
             events=[self._ev("job", SignalStatus.FAILURE, 10)],
@@ -287,7 +297,7 @@ def test_insufficient_successes_returns_restart_newest_success_when_no_pending(
         s = Signal(
             key="job",
             workflow_name="wf",
-            commits=[c_fail_new, c_fail_old, c_success],
+            commits=[c_fail_newest, c_fail_new, c_fail_old, c_success],
         )
         res = s.process_valid_autorevert_pattern()
         self.assertIsNotNone(res)
@@ -298,7 +308,11 @@ def test_insufficient_successes_returns_restart_newest_success_when_no_pending(
         self.assertIn("sha_success", res.commit_shas)
 
     def test_insufficient_successes_infra_not_confirmed_when_pending_on_success(self):
-        # Two failures present, but newest successful has pending → ineligible awaiting confirmation
+        # Three failures present, but newest successful has pending → ineligible awaiting confirmation
+        c_fail_newest = SignalCommit(
+            head_sha="sha_fail_newest",
+            events=[self._ev("job", SignalStatus.FAILURE, 11)],
+        )
         c_fail_new = SignalCommit(
             head_sha="sha_fail_new",
             events=[self._ev("job", SignalStatus.FAILURE, 10)],
@@ -317,13 +331,35 @@ def test_insufficient_successes_infra_not_confirmed_when_pending_on_success(self
         s = Signal(
             key="job",
             workflow_name="wf",
-            commits=[c_fail_new, c_fail_old, c_success_pending],
+            commits=[c_fail_newest, c_fail_new, c_fail_old, c_success_pending],
         )
         res = s.process_valid_autorevert_pattern()
         self.assertIsInstance(res, Ineligible)
         self.assertEqual(res.reason, IneligibleReason.INFRA_NOT_CONFIRMED)
         self.assertIn("sha_success_pend", res.message)
 
+    def test_both_sides_restart_accumulate_when_below_thresholds(self):
+        # One failure total (<3) and one success total (<2), neither pending.
+        # Should propose restarts for both oldest failed and newest successful.
+        c_failed = SignalCommit(
+            head_sha="sha_failed",
+            events=[self._ev("job", SignalStatus.FAILURE, 10)],
+        )
+        c_success = SignalCommit(
+            head_sha="sha_success",
+            events=[self._ev("job", SignalStatus.SUCCESS, 3)],
+        )
+        s = Signal(
+            key="job",
+            workflow_name="wf",
+            commits=[c_failed, c_success],
+        )
+        res = s.process_valid_autorevert_pattern()
+        # Should be a RestartCommits with both SHAs present
+        self.assertTrue(hasattr(res, "commit_shas"))
+        self.assertIn("sha_failed", res.commit_shas)
+        self.assertIn("sha_success", res.commit_shas)
+
 
 if __name__ == "__main__":
     unittest.main()