Judge only the conformance loop on failures that are merely consecutive

The consecutive-failure arm was applied to both loops on one threshold, and the
threshold was calibrated on conformance data because no unit-loop equivalent
existed: the highest failure count on a functionality that then recovered was four,
so six looked safe. It was not. On the unit side it fired on loops that were
working.

Two renders showed the same signature -- unit=7 unit_failed=7 unit_max_repeat=1,
seven failures with none of them alike -- and in both the arm restarted the
functionality and the render scored 0/10. One of them burned its whole restart
budget and gave up in under five minutes. Every render without a unit restart
scored 2 or 3. Seven different failures is a loop working through issues one at a
time; restarting throws all of it away, which is a far worse trade than the one
the conformance side makes by discarding a single test file.

In both harmful cases unit_max_repeat was 1, so the streak arm would never have
fired. That arm stays: across three renders every healthy functionality finished
with unit_max_repeat=1, and nothing was observed between that and the 17 reached by
the one that wedged, so a streak of three is a state healthy renders do not enter.

The conformance loop keeps both arms. There the consecutive arm is what catches a
loop failing 40 times out of 40 with a longest identical run of two, which no
streak threshold reaches, and its remedy costs one regenerated test rather than a
functionality's implementation.

Removing it does not make such a unit loop run forever; it stops where it always
did, at the attempt limit.

Author: gorandodig

render_machine/fix_loop_metrics.py

@@ -168,20 +168,36 @@ def render_summary(self) -> List[str]:
 STRATEGY_SWITCH_PREFIX = "[strategy-switch]"
 
 
+# Which loops a run of failures — as opposed to a run of *identical* failures — is taken
+# as evidence against. Conformance only, and the asymmetry is measured rather than
+# cautious. On the conformance side the arm is what catches a loop failing 40 times out of
+# 40 with a longest identical run of two, which no streak threshold reaches. On the unit
+# side it fired on loops that were working: two renders showed `unit=7 unit_failed=7
+# unit_max_repeat=1` — seven failures, none alike — and both had their functionality
+# restarted and scored 0/10, where every render without a restart scored 2–3. Six was
+# calibrated on conformance recoveries (the highest failure count on a functionality that
+# then recovered was four) and there was never a unit-loop equivalent to calibrate on.
+#
+# The unit loop keeps the streak arm, where the margin is not in doubt: across three
+# renders every healthy functionality finished with unit_max_repeat=1.
+LOOPS_JUDGED_ON_CONSECUTIVE_FAILURES = (CONFORMANCE_LOOP,)
+
+
 def stalled_reason(metrics: "FixLoopMetrics", loop: str, module: str, frid: Optional[str]) -> Optional[str]:
     """Why this loop looks stuck, or None if it still looks like it is working.
 
-    One definition for both loops. The streak arm fires soonest when a loop is
-    re-submitting the same fix; the consecutive arm is the catch-all for a loop that
-    fails every time while the failures keep changing shape.
+    The streak arm applies to both loops: a loop re-submitting the same fix is stuck
+    whichever loop it is. The consecutive arm applies only where failing every time has
+    been shown to mean stuck rather than busy.
     """
     streak = metrics.current_streak(loop, module, frid)
     if streak >= REPEATED_FAILURE_WARNING_THRESHOLD:
         return f"repeated_failure streak={streak}"
 
-    failures = metrics.consecutive_failures(loop, module, frid)
-    if failures >= CONSECUTIVE_FAILURE_THRESHOLD:
-        return f"no_progress consecutive_failures={failures}"
+    if loop in LOOPS_JUDGED_ON_CONSECUTIVE_FAILURES:
+        failures = metrics.consecutive_failures(loop, module, frid)
+        if failures >= CONSECUTIVE_FAILURE_THRESHOLD:
+            return f"no_progress consecutive_failures={failures}"
 
     return None
 

tests/test_unit_strategy_switch.py

@@ -2,12 +2,16 @@
 
 The unit loop has the same problem as the conformance one and a different remedy: its
 escape hatch restarts the functionality from scratch rather than discarding a test file.
-That is destructive enough to be worth a careful threshold, and the benchmark data makes
-the call easy — across three renders every healthy functionality finished with
-`unit_max_repeat=1`, nothing was ever observed between that and the 17 reached by the one
-that wedged. So a streak of three is a state healthy renders do not enter, and reaching
-the same decision on it saves the eleven minutes that render spent grinding to the
-attempt limit.
+That is destructive enough that only one kind of evidence justifies it. Repetition does:
+across three renders every healthy functionality finished with `unit_max_repeat=1`, and
+nothing was observed between that and the 17 reached by the one that wedged, so a streak
+of three is a state healthy renders do not enter.
+
+A run of failures that are merely consecutive does not. That arm was applied here too at
+first, calibrated on conformance recoveries because no unit-loop equivalent existed, and
+it fired on loops that were working: `unit=7 unit_failed=7 unit_max_repeat=1` in two
+renders, both restarted, both 0/10, against 2-3 for every render without a restart. It now
+applies to the conformance loop only.
 """
 
 from unittest.mock import MagicMock, patch
@@ -62,9 +66,30 @@ def test_a_healthy_loop_is_left_alone():
     assert gave_up(context(identical_failures=0)) is False
 
 
-def test_a_unit_loop_that_always_fails_gives_up_without_a_repeat():
+def test_a_unit_loop_failing_every_time_but_differently_is_left_alone():
+    """Measured, not cautious. Two renders showed `unit=7 unit_failed=7
+    unit_max_repeat=1` — seven failures, none alike — and both had their functionality
+    restarted and scored 0/10, where every render without a restart scored 2-3. Seven
+    different failures is a loop working through issues one at a time, and restarting
+    discards all of it.
+
+    The conformance loop keeps this arm; there, failing every time really does mean
+    stuck, and it is what catches a 40-out-of-40 run whose longest identical streak is
+    two."""
     instance = context()
-    for index in range(CONSECUTIVE_FAILURE_THRESHOLD):
+    for index in range(CONSECUTIVE_FAILURE_THRESHOLD * 2):
+        instance.fix_loop_metrics.record(
+            UNIT_LOOP, module=MODULE, frid=FRID, passed=False, output=f"failure number {index}"
+        )
+
+    assert gave_up(instance) is False
+
+
+def test_the_attempt_limit_still_catches_a_unit_loop_that_never_repeats():
+    """Removing the arm does not make such a loop run forever: it stops where it always
+    did, at the attempt limit."""
+    instance = context(attempts=MAX_UNITTEST_FIX_ATTEMPTS)
+    for index in range(CONSECUTIVE_FAILURE_THRESHOLD * 2):
         instance.fix_loop_metrics.record(
             UNIT_LOOP, module=MODULE, frid=FRID, passed=False, output=f"failure number {index}"
         )