From 290e9799f69b06ec55cac3ac238f548dd87c8598 Mon Sep 17 00:00:00 2001 From: Goran Dodig Date: Thu, 20 Aug 2026 07:35:04 +0200 Subject: [PATCH 1/2] Detect a stalled fix loop Both fix loops patch until they hit the attempt limit, including when the patches are having no effect. The instrumentation made that visible; nothing acted on it. A loop is now treated as stalled on either of two signals: - The same failure repeats. Identical fingerprints in a row mean the last patches changed nothing the test can observe. - Every attempt fails, with the failures varying. No streak counter detects this at any threshold. The second signal applies to the conformance loop only. Applied to the unit loop it produced false positives: a run of differing failures there is usually the loop working through issues one at a time, and the unit loop's remedy is to restart the functionality from scratch, so a false positive is expensive. The unit loop keeps the repeated-failure signal. Reaching it gives the same answer it would have got at the attempt limit, sooner. The stall counters reset when a loop is handed different work, such as a regenerated conformance test. Otherwise the replacement's first failure lands on a counter already over the threshold and the replacement is discarded after one attempt. Cumulative counts are kept, since they answer a different question. --- .../actions/fix_conformance_test.py | 77 ++++++++- render_machine/fix_loop_metrics.py | 95 +++++++++++ render_machine/render_context.py | 48 +++++- tests/test_fix_loop_metrics.py | 78 ++++++++- tests/test_unit_strategy_switch.py | 154 ++++++++++++++++++ 5 files changed, 447 insertions(+), 5 deletions(-) create mode 100644 tests/test_unit_strategy_switch.py diff --git a/render_machine/actions/fix_conformance_test.py b/render_machine/actions/fix_conformance_test.py index 5d68cc88..1a42dbd8 100644 --- a/render_machine/actions/fix_conformance_test.py +++ b/render_machine/actions/fix_conformance_test.py @@ -7,12 +7,31 @@ from plain2code_console import RETRY_COLOR, console from plain2code_exceptions import InternalClientError from render_machine.actions.base_action import BaseAction +from render_machine.fix_loop_metrics import CONFORMANCE_LOOP, STRATEGY_SWITCH_PREFIX, stalled_reason from render_machine.implementation_code_helpers import ImplementationCodeHelpers from render_machine.render_context import RenderContext -from render_machine.render_types import RenderError, TestExecutionPhase +from render_machine.render_types import AcceptanceTestPhase, RenderError, TestExecutionPhase MAX_CONFORMANCE_TEST_FIX_ATTEMPTS = 20 -MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS = 1 + +# How many times one functionality may have its conformance test regenerated before the +# loop falls back to patching until the attempt limit. The budget is per functionality — +# `ConformanceTestsRunningContext` is rebuilt for each one — so this is not a per-render +# allowance. +# +# One is not enough. The attempt limit below is only reachable once this budget is spent, +# so with a budget of one a functionality needing a second regeneration is abandoned +# instead: one regeneration, then twenty patches that change nothing, then failure. +# Renders that completed used one regeneration on each of several functionalities; renders +# that did not reached a functionality that needed two. Regeneration discards a test the +# loop has already shown it cannot satisfy, so stopping at the first one gives up exactly +# where the move is working. +# +# Three rather than more: each regeneration resets `fix_attempts`, so the worst case for +# an unfixable functionality is four rounds of patching instead of two, and that cost +# falls on renders that were going to fail anyway. Raise it further only on evidence that +# a fourth regeneration helps. +MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS = 3 class FixConformanceTest(BaseAction): @@ -26,6 +45,56 @@ class FixConformanceTest(BaseAction): ISSUE_REASON_CODE_CONFLICTING_REQUIREMENTS = 2 ISSUE_REASON_CODE_CONFLICTING_ACCEPTANCE_TESTS = 3 + @staticmethod + def _should_regenerate_instead_of_patching(render_context: RenderContext) -> bool: + """Whether the loop has proven that patching the implementation is not working. + + Two kinds of stall, both seen on real renders: a failure that repeats identically, + meaning the last patches changed nothing the test can observe; and a loop that + fails every attempt while the failures keep changing, which no streak threshold + detects. `stalled_reason` covers both. + + Regenerating the conformance test is a different move: it discards the test the + loop cannot satisfy instead of editing code against it again. That path already + exists for the attempt limit; this reaches it as soon as there is evidence rather + than after twenty patches. + """ + ctx = render_context.conformance_tests_running_context + + # Bounded by the same re-render budget as the attempt-limit path, so the switch + # cannot cycle: once it is spent, the loop patches until the limit and stops. + if ctx.conformance_tests_render_attempts >= MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS: + return False + + # Only a test this module owns, outside the incremental acceptance phase. + # Regeneration renders the replacement for the current module, so a required + # module's test returns under the wrong prefix and an acceptance phase left + # mid-flight looks up an entry that is gone. + if ctx.current_testing_module_name != render_context.module_name: + return False + if ctx.acceptance_test_phase is not AcceptanceTestPhase.NOT_STARTED: + return False + + reason = stalled_reason( + render_context.fix_loop_metrics, + CONFORMANCE_LOOP, + module=render_context.module_name, + frid=ctx.current_testing_frid, + ) + if reason is None: + return False + + console.warning( + f"{STRATEGY_SWITCH_PREFIX} module={render_context.module_name} " + f"frid={ctx.current_testing_frid} loop={CONFORMANCE_LOOP} {reason} " + f"action=regenerate_conformance_tests" + ) + console.info( + f"Patching the implementation has not made the conformance tests for functionality " + f"{ctx.current_testing_frid} pass ({reason}). Regenerating those tests instead." + ) + return True + def execute(self, render_context: RenderContext, previous_action_payload: Any | None): ctx = render_context.conformance_tests_running_context ctx.fix_attempts += 1 @@ -42,6 +111,10 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | ctx.regenerating_conformance_tests = True return self.REGENERATE_CONFORMANCE_TESTS_OUTCOME, None + if self._should_regenerate_instead_of_patching(render_context): + ctx.regenerating_conformance_tests = True + return self.REGENERATE_CONFORMANCE_TESTS_OUTCOME, None + console.info(f"Running conformance tests attempt {ctx.fix_attempts + 1}.") console.info( diff --git a/render_machine/fix_loop_metrics.py b/render_machine/fix_loop_metrics.py index beb60b0f..749325ba 100644 --- a/render_machine/fix_loop_metrics.py +++ b/render_machine/fix_loop_metrics.py @@ -60,6 +60,10 @@ class _LoopCounters: max_repeat: int = 1 last_fingerprint: Optional[str] = None current_repeat: int = 0 + # Failures since the last pass, whether or not they look alike. A loop can fail every + # attempt while the failure keeps changing, which a streak counter cannot detect at + # any threshold. + consecutive_failures: int = 0 @dataclass @@ -83,9 +87,11 @@ def record(self, loop: str, module: str, frid: str, passed: bool, output: str) - if passed: counters.last_fingerprint = None counters.current_repeat = 0 + counters.consecutive_failures = 0 return None counters.failures += 1 + counters.consecutive_failures += 1 fingerprint = failure_fingerprint(output) if fingerprint == counters.last_fingerprint: counters.current_repeat += 1 @@ -96,6 +102,50 @@ def record(self, loop: str, module: str, frid: str, passed: bool, output: str) - counters.current_repeat = 1 return None + def start_over(self, loop: str, module: str, frid: Optional[str]) -> None: + """Clears the stall counters, keeping the cumulative ones. + + Called when the loop is given different work - a regenerated conformance test - + rather than another patch against the same one. The stall evidence describes a + test that no longer exists. Keeping it means the replacement's first failure lands + on a counter already over the threshold, so the replacement is discarded after one + attempt and the regeneration budget is spent without any replacement being tried. + + Cumulative counts survive: they answer a different question, how much work the + functionality took, and reporting is indexed on them. + """ + counters = self._counters_for(loop, module, frid) + if counters is None: + return + counters.consecutive_failures = 0 + counters.current_repeat = 0 + counters.last_fingerprint = None + + def current_streak(self, loop: str, module: str, frid: Optional[str]) -> int: + """How many times in a row this loop has just failed the same way. + + `record` returns the streak as it happens, which is enough to warn but not to + decide: the fix action runs after the test action and needs to ask the question + again, from its own call site. A missing frid answers zero rather than raising, + because nothing was recorded under one either — `report_fix_loop_attempt` skips + those runs. + """ + counters = self._counters_for(loop, module, frid) + return counters.current_repeat if counters else 0 + + def consecutive_failures(self, loop: str, module: str, frid: Optional[str]) -> int: + """How many times in a row this loop has failed, regardless of how it failed.""" + counters = self._counters_for(loop, module, frid) + return counters.consecutive_failures if counters else 0 + + def _counters_for(self, loop: str, module: str, frid: Optional[str]) -> Optional[_LoopCounters]: + if frid is None: + return None # nothing is recorded without one — report_fix_loop_attempt skips those runs + counters = self._counters.get((module, str(frid))) + if not counters or loop not in counters: + return None + return counters[loop] + def frid_summary(self, module: str, frid: str) -> Optional[str]: """One greppable line per FRID, or None if no script ran for it.""" counters = self._counters.get((module, str(frid))) @@ -138,6 +188,51 @@ def render_summary(self) -> List[str]: # re-patching against a failure it is not moving. REPEATED_FAILURE_WARNING_THRESHOLD = 3 +# How many failures in a row - alike or not - before the loop is called stalled anyway. +# A repeated failure proves futility quickly, but a loop can also fail every attempt while +# the failure keeps changing, which no streak threshold detects. Set above the highest +# failure count seen on a functionality that then recovered, so a loop that is slow but +# converging is not cut short. +CONSECUTIVE_FAILURE_THRESHOLD = 6 + +# Marks the point where a loop stops patching and does something else. Greppable on +# purpose, like the other machine-read markers. +STRATEGY_SWITCH_PREFIX = "[strategy-switch]" + + +# Which loops treat a run of failures - as opposed to a run of *identical* failures - as +# evidence of a stall. Conformance only. +# +# On the conformance side it catches a loop that fails every attempt while the failure +# keeps changing, which the streak signal cannot see. +# +# On the unit side it produced false positives. A run of differing failures in the unit +# loop is usually the loop working through issues one at a time, and the unit remedy is to +# restart the functionality from scratch, discarding that progress. The threshold was also +# calibrated on conformance recoveries, with no unit-loop equivalent to calibrate against. +# +# Both loops keep the streak signal, where a healthy loop does not repeat a failure. +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 stalled, or None if it still looks like it is working. + + The repeated-failure signal applies to both loops: a loop re-submitting the same fix + is stalled whichever loop it is. The consecutive-failure signal applies only where + failing every attempt has been shown to mean stalled rather than busy. + """ + streak = metrics.current_streak(loop, module, frid) + if streak >= REPEATED_FAILURE_WARNING_THRESHOLD: + return f"repeated_failure streak={streak}" + + 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 + def report_fix_loop_attempt(render_context, loop: str, frid: Optional[str], passed: bool, output: str) -> None: """Records one script run and tells the user when the loop stops making progress.""" diff --git a/render_machine/render_context.py b/render_machine/render_context.py index 3ce88595..c5c9b422 100644 --- a/render_machine/render_context.py +++ b/render_machine/render_context.py @@ -13,7 +13,13 @@ from plain_modules import PlainModule from render_machine import triggers from render_machine.conformance_tests import CONFORMANCE_TESTS_DEFINITION_FILE_NAME, ConformanceTests -from render_machine.fix_loop_metrics import FixLoopMetrics +from render_machine.fix_loop_metrics import ( + CONFORMANCE_LOOP, + STRATEGY_SWITCH_PREFIX, + UNIT_LOOP, + FixLoopMetrics, + stalled_reason, +) from render_machine.render_types import ( AcceptanceTestPhase, ConformanceTestsRunningContext, @@ -269,9 +275,43 @@ def finish_unittests_processing(self): self.unit_tests_running_context.fix_attempts = 1 def start_fixing_unit_tests(self, on_limit_exceeded: Callable): + def restart() -> None: + """Hands the functionality back for a fresh attempt, and forgets the stall. + + The restart discards the code the streak was measured against. Left standing, + the fresh attempt's first failure is already past the threshold and restarts + again before the fix loop gets one attempt. Cumulative counts survive. + """ + self.fix_loop_metrics.start_over( + UNIT_LOOP, + module=self.module_name, + frid=self.frid_context.frid if self.frid_context else None, + ) + on_limit_exceeded() + self.unit_tests_running_context.fix_attempts += 1 if self.unit_tests_running_context.fix_attempts > MAX_UNITTEST_FIX_ATTEMPTS: - on_limit_exceeded() + restart() + return + + # A stalled unit loop gets the same answer as one that ran out of attempts, just + # sooner. Only the repeated-failure signal is used here: a healthy unit loop does + # not repeat the same failure, so a short streak is a reliable indicator, and the + # remedy - restarting the functionality from scratch - is too destructive to + # trigger on the weaker signal. + reason = stalled_reason( + self.fix_loop_metrics, + UNIT_LOOP, + module=self.module_name, + frid=self.frid_context.frid if self.frid_context else None, + ) + if reason is not None: + console.warning( + f"{STRATEGY_SWITCH_PREFIX} module={self.module_name} " + f"frid={self.frid_context.frid if self.frid_context else None} loop={UNIT_LOOP} " + f"{reason} action=give_up_on_patching" + ) + restart() def _on_unit_test_limit_exceeded_in_implementation(self): self.machine.dispatch(triggers.RESTART_FRID_PROCESSING) @@ -395,6 +435,10 @@ def _handle_test_regeneration(self): ctx.conformance_tests_render_attempts += 1 ctx.fix_attempts = 0 ctx.regenerating_conformance_tests = False + # The stall that triggered this was measured against the test just deleted. Left + # standing, it condemns the replacement on its first failure and the whole + # regeneration budget is spent in seconds on tests that never got a second look. + self.fix_loop_metrics.start_over(CONFORMANCE_LOOP, module=self.module_name, frid=ctx.current_testing_frid) def _handle_retry_after_code_change(self): """Re-run the test that failed and triggered a code change.""" diff --git a/tests/test_fix_loop_metrics.py b/tests/test_fix_loop_metrics.py index 712cb669..17d36e7d 100644 --- a/tests/test_fix_loop_metrics.py +++ b/tests/test_fix_loop_metrics.py @@ -14,7 +14,14 @@ import pytest -from render_machine.fix_loop_metrics import CONFORMANCE_LOOP, UNIT_LOOP, FixLoopMetrics, failure_fingerprint +from render_machine.fix_loop_metrics import ( + CONFORMANCE_LOOP, + CONSECUTIVE_FAILURE_THRESHOLD, + UNIT_LOOP, + FixLoopMetrics, + failure_fingerprint, + stalled_reason, +) def test_the_same_failure_fingerprints_the_same(): @@ -128,6 +135,42 @@ def test_each_loop_reports_its_own_streak(): assert "max_repeat=7" in summary # the aggregate stays, for continuity of the series +def test_the_current_streak_is_readable_after_the_fact(): + """The fix action runs after the test action and has to ask again, from its own call + site, rather than relying on what record() returned to someone else.""" + metrics = FixLoopMetrics() + for _ in range(3): + metrics.record(CONFORMANCE_LOOP, module="m", frid="1", passed=False, output="same") + + assert metrics.current_streak(CONFORMANCE_LOOP, "m", "1") == 3 + + +def test_the_current_streak_resets_when_the_failure_changes(): + metrics = FixLoopMetrics() + for _ in range(3): + metrics.record(CONFORMANCE_LOOP, module="m", frid="1", passed=False, output="same") + metrics.record(CONFORMANCE_LOOP, module="m", frid="1", passed=False, output="different") + + assert metrics.current_streak(CONFORMANCE_LOOP, "m", "1") == 1 + + +def test_the_current_streak_clears_when_the_loop_passes(): + metrics = FixLoopMetrics() + for _ in range(3): + metrics.record(CONFORMANCE_LOOP, module="m", frid="1", passed=False, output="same") + metrics.record(CONFORMANCE_LOOP, module="m", frid="1", passed=True, output="") + + assert metrics.current_streak(CONFORMANCE_LOOP, "m", "1") == 0 + + +def test_an_unrun_loop_has_no_streak(): + metrics = FixLoopMetrics() + metrics.record(UNIT_LOOP, module="m", frid="1", passed=False, output="boom") + + assert metrics.current_streak(CONFORMANCE_LOOP, "m", "1") == 0 + assert metrics.current_streak(CONFORMANCE_LOOP, "m", "9") == 0 + + def test_an_unseen_frid_has_no_summary(): assert FixLoopMetrics().frid_summary("m", "9") is None @@ -178,3 +221,36 @@ def test_a_frid_reports_its_summary_only_once(): assert metrics.take_frid_summary("m", "1") is not None assert metrics.take_frid_summary("m", "1") is None assert metrics.render_summary() == [] + + +def test_a_regenerated_test_is_not_condemned_by_the_old_test_s_stall(): + """Regeneration hands the loop a different test. If the stall measured against the + deleted one survives, the replacement is discarded on its first failure.""" + metrics = FixLoopMetrics() + for _ in range(CONSECUTIVE_FAILURE_THRESHOLD): + metrics.record(CONFORMANCE_LOOP, module="m", frid="2", passed=False, output="always different %s") + + assert stalled_reason(metrics, CONFORMANCE_LOOP, module="m", frid="2") is not None + + metrics.start_over(CONFORMANCE_LOOP, module="m", frid="2") + + assert stalled_reason(metrics, CONFORMANCE_LOOP, module="m", frid="2") is None + + # One failure of the replacement must not re-trigger the switch on its own. + metrics.record(CONFORMANCE_LOOP, module="m", frid="2", passed=False, output="a new failure") + + assert stalled_reason(metrics, CONFORMANCE_LOOP, module="m", frid="2") is None + + +def test_starting_over_keeps_the_work_already_counted(): + """The cumulative counts answer a different question, so a reset must not erase them.""" + metrics = FixLoopMetrics() + for _ in range(4): + metrics.record(CONFORMANCE_LOOP, module="m", frid="2", passed=False, output="identical") + + metrics.start_over(CONFORMANCE_LOOP, module="m", frid="2") + summary = metrics.frid_summary("m", "2") + + assert "conformance=4" in summary + assert "conformance_failed=4" in summary + assert "conformance_max_repeat=4" in summary diff --git a/tests/test_unit_strategy_switch.py b/tests/test_unit_strategy_switch.py new file mode 100644 index 00000000..5a9d606e --- /dev/null +++ b/tests/test_unit_strategy_switch.py @@ -0,0 +1,154 @@ +"""Tests for giving up on patching when the unit fix loop stops making progress. + +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 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 needlessly restarted, both producing worse output than the renders that +were left alone. It now +applies to the conformance loop only. +""" + +from unittest.mock import MagicMock, patch + +import render_machine.render_context as render_context_module +from render_machine.fix_loop_metrics import ( + CONFORMANCE_LOOP, + CONSECUTIVE_FAILURE_THRESHOLD, + REPEATED_FAILURE_WARNING_THRESHOLD, + STRATEGY_SWITCH_PREFIX, + UNIT_LOOP, + FixLoopMetrics, + stalled_reason, +) +from render_machine.render_context import MAX_UNITTEST_FIX_ATTEMPTS, RenderContext + +MODULE = "vault_cli" +FRID = "2" + + +def context(identical_failures=0, attempts=1, output="AssertionError: vault not initialized"): + instance = MagicMock(spec=RenderContext) + instance.module_name = MODULE + instance.fix_loop_metrics = FixLoopMetrics() + instance.frid_context = MagicMock() + instance.frid_context.frid = FRID + instance.unit_tests_running_context = MagicMock() + instance.unit_tests_running_context.fix_attempts = attempts + for _ in range(identical_failures): + instance.fix_loop_metrics.record(UNIT_LOOP, module=MODULE, frid=FRID, passed=False, output=output) + return instance + + +def gave_up(instance): + """Whether start_fixing_unit_tests reached the give-up handler.""" + on_limit_exceeded = MagicMock() + with patch.object(render_context_module, "console"): + RenderContext.start_fixing_unit_tests(instance, on_limit_exceeded) + return on_limit_exceeded.called + + +def test_a_unit_loop_repeating_a_failure_three_times_gives_up_early(): + assert gave_up(context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD)) is True + + +def test_two_repeats_are_left_alone(): + """No healthy functionality observed ever reached two, so this is + already past normal — but the remedy discards the whole functionality, so it waits + for the same evidence the conformance side does.""" + assert gave_up(context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD - 1)) is False + + +def test_a_healthy_loop_is_left_alone(): + assert gave_up(context(identical_failures=0)) is False + + +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, producing worse output than every render that was left alone. 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 * 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}" + ) + + assert gave_up(instance) is True + + +def test_a_stuck_conformance_loop_does_not_restart_the_functionality(): + """The conformance loop has its own, far cheaper remedy; it must not reach this one.""" + instance = context() + for _ in range(8): + instance.fix_loop_metrics.record(CONFORMANCE_LOOP, module=MODULE, frid=FRID, passed=False, output="same") + + assert gave_up(instance) is False + + +def test_the_attempt_limit_still_ends_the_loop_on_its_own(): + """The streak arm is an early exit, not a replacement: a loop that never repeats and + never accumulates enough consecutive failures still stops at the limit.""" + assert gave_up(context(attempts=MAX_UNITTEST_FIX_ATTEMPTS)) is True + + +def test_the_early_exit_is_announced_in_a_greppable_form(): + instance = context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD + 1) + + with patch.object(render_context_module, "console") as console: + RenderContext.start_fixing_unit_tests(instance, MagicMock()) + + announced = console.warning.call_args[0][0] + assert STRATEGY_SWITCH_PREFIX in announced + assert f"module={MODULE}" in announced + assert f"frid={FRID}" in announced + assert "loop=unit" in announced + assert f"repeated_failure streak={REPEATED_FAILURE_WARNING_THRESHOLD + 1}" in announced + assert "action=give_up_on_patching" in announced + + +def test_hitting_the_attempt_limit_is_not_announced_as_a_switch(): + """The limit path is ordinary exhaustion, not a decision the loop made about its own + progress; labelling it a strategy switch would inflate every strategy-switch count.""" + instance = context(attempts=MAX_UNITTEST_FIX_ATTEMPTS) + + with patch.object(render_context_module, "console") as console: + RenderContext.start_fixing_unit_tests(instance, MagicMock()) + + assert not console.warning.called + + +def test_restarting_a_functionality_clears_its_stall_counters(): + """The restart discards the code the streak was measured against. If the counters + survive, the fresh attempt restarts again before the fix loop gets one attempt.""" + instance = context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD) + + with patch.object(render_context_module, "console"): + RenderContext.start_fixing_unit_tests(instance, MagicMock()) + + assert stalled_reason(instance.fix_loop_metrics, UNIT_LOOP, module=MODULE, frid=FRID) is None + # The cumulative work still counts; only the evidence of being stuck is dropped. + assert f"{UNIT_LOOP}_failed=3" in instance.fix_loop_metrics.frid_summary(MODULE, FRID) From 82a6d2a1f53caceb0559f80a811c09327520a880 Mon Sep 17 00:00:00 2001 From: Goran Dodig Date: Thu, 20 Aug 2026 07:35:20 +0200 Subject: [PATCH 2/2] Regenerate the conformance test when the loop has stalled Once the conformance loop is stalled, patching the implementation again is the move already shown not to work. Discarding the conformance test and generating a new one is a different move. That path existed, but only at the attempt limit, after twenty patches that changed nothing. It is now taken as soon as the loop is judged stalled. The regeneration budget goes from one to three. One is not enough: the attempt limit is only reachable once the budget is spent, so a functionality needing a second regeneration is abandoned instead. Renders that completed used one regeneration on each of several functionalities; renders that did not reached a functionality that needed two. Three rather than more because each regeneration resets the attempt count. The worst case for an unfixable functionality is four rounds of patching instead of two, and that cost falls on renders that were going to fail anyway. Raising it further needs evidence that a fourth regeneration helps. --- tests/test_conformance_strategy_switch.py | 234 ++++++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 tests/test_conformance_strategy_switch.py diff --git a/tests/test_conformance_strategy_switch.py b/tests/test_conformance_strategy_switch.py new file mode 100644 index 00000000..674d4cfc --- /dev/null +++ b/tests/test_conformance_strategy_switch.py @@ -0,0 +1,234 @@ +"""Tests for switching strategy when the conformance fix loop stops making progress. + +The loop's failure mode is not slowness, and it comes in two shapes. It can re-send the +same fix request and get back a patch that changes nothing the test can see — a wedged +render failed conformance 20 times on one functionality with a streak of 8 while its unit +loop never failed once. Or it can fail every single time while the failures keep changing +shape — another went 40 for 40 with a longest identical run of two, which no streak +threshold can catch. Both burn the whole budget. + +Regenerating the conformance test is the different move — it discards the test the loop +cannot satisfy instead of editing code against it again. These tests pin when that +happens, and just as importantly when it does not: both thresholds have to sit above what +a healthy functionality does, or every good render pays for it. +""" + +from unittest.mock import MagicMock, patch + +from render_machine.actions.fix_conformance_test import MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS, FixConformanceTest +from render_machine.fix_loop_metrics import ( + CONFORMANCE_LOOP, + CONSECUTIVE_FAILURE_THRESHOLD, + REPEATED_FAILURE_WARNING_THRESHOLD, + STRATEGY_SWITCH_PREFIX, + UNIT_LOOP, + FixLoopMetrics, +) +from render_machine.render_types import AcceptanceTestPhase + +MODULE = "vault_cli" +FRID = "2" + + +def render_context(identical_failures=0, render_attempts=0, output="AssertionError: prompt not shown"): + context = MagicMock() + context.module_name = MODULE + context.fix_loop_metrics = FixLoopMetrics() + for _ in range(identical_failures): + context.fix_loop_metrics.record(CONFORMANCE_LOOP, module=MODULE, frid=FRID, passed=False, output=output) + + ctx = context.conformance_tests_running_context + ctx.current_testing_frid = FRID + ctx.current_testing_module_name = MODULE + ctx.conformance_tests_render_attempts = render_attempts + ctx.fix_attempts = 4 # mid-loop: well below the attempt limit + ctx.regenerating_conformance_tests = False + ctx.acceptance_test_phase = AcceptanceTestPhase.NOT_STARTED + return context + + +def decides_to_regenerate(context): + with patch("render_machine.actions.fix_conformance_test.console"): + return FixConformanceTest._should_regenerate_instead_of_patching(context) + + +def test_a_loop_that_repeats_a_failure_three_times_regenerates_the_test(): + assert decides_to_regenerate(render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD)) is True + + +def test_a_healthy_functionality_that_repeats_twice_is_left_alone(): + """Observed in a real render: a functionality repeated a failure twice and then + converged. A threshold of two would abandon tests that were about to pass.""" + assert REPEATED_FAILURE_WARNING_THRESHOLD >= 2, "one below the threshold has to be a real repeat" + assert decides_to_regenerate(render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD - 1)) is False + + +def test_a_first_failure_does_not_trigger_it(): + assert decides_to_regenerate(render_context(identical_failures=1)) is False + + +def test_a_loop_that_has_not_run_yet_does_not_trigger_it(): + assert decides_to_regenerate(render_context(identical_failures=0)) is False + + +def test_failures_that_differ_do_not_count_as_repeats(): + """A loop making progress produces new failures; only identical ones prove it is + stuck.""" + context = render_context(identical_failures=0) + for output in ("first failure", "second failure", "third failure"): + context.fix_loop_metrics.record(CONFORMANCE_LOOP, module=MODULE, frid=FRID, passed=False, output=output) + + assert decides_to_regenerate(context) is False + + +def test_a_render_with_no_functionality_under_test_does_not_trigger_it(): + """current_testing_frid is optional; nothing is recorded under a missing one.""" + context = render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD) + context.conformance_tests_running_context.current_testing_frid = None + + assert decides_to_regenerate(context) is False + + +def test_a_stuck_unit_loop_does_not_regenerate_conformance_tests(): + """The two loops fail for different reasons and warrant different responses.""" + context = render_context(identical_failures=0) + for _ in range(5): + context.fix_loop_metrics.record(UNIT_LOOP, module=MODULE, frid=FRID, passed=False, output="same") + + assert decides_to_regenerate(context) is False + + +def test_the_switch_stops_once_its_budget_is_spent_and_cannot_cycle(): + """Regeneration draws on the same budget as the attempt-limit path. Once it is spent + the loop patches to the limit and stops, rather than regenerating forever.""" + spent = render_context( + identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD + 1, + render_attempts=MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS, + ) + + assert decides_to_regenerate(spent) is False + + +def test_a_functionality_may_be_regenerated_more_than_once(): + """The budget that mattered. Every render that produced no code died at the attempt + limit, reachable only after this budget ran out — one regeneration, then twenty + fruitless patches. A render that completed needed one regeneration on each of three + functionalities; the ones that wedged needed a second on a single functionality and had + none. Stopping after the first abandons the render where the move is still working.""" + # Deliberately not `range(1, MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS)`: a bound derived + # from the constant makes the test vacuous at the value it is meant to rule out, and it + # passed against a budget of 1 for exactly that reason. One is the count that has to be + # named literally here, because one is what the wedged renders got. + already_regenerated_once = render_context( + identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD + 1, render_attempts=1 + ) + + assert decides_to_regenerate(already_regenerated_once) is True + + assert MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS >= 2, ( + "the budget has to allow a second regeneration for the assertion above to mean " + "anything; at 1 the loop abandons a functionality the move was still working on" + ) + + +def test_the_switch_is_announced_in_a_greppable_form(): + """These logs are read by tooling before they are read by a person.""" + context = render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD + 1) + + with patch("render_machine.actions.fix_conformance_test.console") as console: + FixConformanceTest._should_regenerate_instead_of_patching(context) + + announced = console.warning.call_args[0][0] + assert STRATEGY_SWITCH_PREFIX in announced + assert f"module={MODULE}" in announced + assert f"frid={FRID}" in announced + assert "loop=conformance" in announced + assert f"repeated_failure streak={REPEATED_FAILURE_WARNING_THRESHOLD + 1}" in announced + assert "action=regenerate_conformance_tests" in announced + + +def failing_differently(context, times): + for index in range(times): + context.fix_loop_metrics.record( + CONFORMANCE_LOOP, module=MODULE, frid=FRID, passed=False, output=f"failure number {index}" + ) + return context + + +def test_a_loop_that_always_fails_regenerates_even_without_a_repeat(): + """The case a streak trigger cannot see at any threshold: a render failed a + functionality's conformance tests 40 times out of 40 while its longest + identical run was two, exhausted its whole budget, and the switch stayed silent.""" + context = failing_differently(render_context(), CONSECUTIVE_FAILURE_THRESHOLD) + + assert decides_to_regenerate(context) is True + + +def test_a_loop_short_of_the_consecutive_bound_is_left_alone(): + context = failing_differently(render_context(), CONSECUTIVE_FAILURE_THRESHOLD - 1) + + assert decides_to_regenerate(context) is False + + +def test_a_pass_clears_the_consecutive_count(): + """A loop that gets a test passing is making progress, however many failures it took + to get there.""" + context = failing_differently(render_context(), CONSECUTIVE_FAILURE_THRESHOLD) + context.fix_loop_metrics.record(CONFORMANCE_LOOP, module=MODULE, frid=FRID, passed=True, output="") + failing_differently(context, 1) + + assert decides_to_regenerate(context) is False + + +def test_the_consecutive_arm_is_announced_with_its_own_reason(): + """The two arms mean different things to a reader, so the marker distinguishes + them rather than reporting one cause for both.""" + context = failing_differently(render_context(), CONSECUTIVE_FAILURE_THRESHOLD) + + with patch("render_machine.actions.fix_conformance_test.console") as console: + FixConformanceTest._should_regenerate_instead_of_patching(context) + + announced = console.warning.call_args[0][0] + assert f"no_progress consecutive_failures={CONSECUTIVE_FAILURE_THRESHOLD}" in announced + + +def test_the_action_returns_the_regeneration_outcome_and_marks_the_context(): + """The early return has to reach the state machine the same way the attempt-limit + path does, or the render carries on patching regardless of the decision.""" + context = render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD) + + with patch("render_machine.actions.fix_conformance_test.console"): + outcome, payload = FixConformanceTest().execute(context, {"previous_conformance_tests_issue": "boom"}) + + assert outcome == FixConformanceTest.REGENERATE_CONFORMANCE_TESTS_OUTCOME + assert payload is None + assert context.conformance_tests_running_context.regenerating_conformance_tests is True + + +def test_the_api_is_not_asked_for_another_patch_when_switching(): + """The point of the switch is to stop spending fix requests on a test the loop + cannot satisfy.""" + context = render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD) + + with patch("render_machine.actions.fix_conformance_test.console"): + FixConformanceTest().execute(context, {"previous_conformance_tests_issue": "boom"}) + + context.codeplain_api.fix_conformance_tests_issue.assert_not_called() + + +def test_a_required_modules_test_is_not_regenerated_by_the_switch(): + """The replacement is rendered under the current module's folder while the required + module stays under test, so the next lookup rejects the prefix.""" + context = render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD + 1) + context.conformance_tests_running_context.current_testing_module_name = "a_required_module" + + assert decides_to_regenerate(context) is False + + +def test_an_acceptance_phase_in_flight_is_not_regenerated_by_the_switch(): + """Regeneration removes the entry and its folder; the incremental phase then looks up + an entry that is gone.""" + context = render_context(identical_failures=REPEATED_FAILURE_WARNING_THRESHOLD + 1) + context.conformance_tests_running_context.acceptance_test_phase = AcceptanceTestPhase.IN_PROGRESS + + assert decides_to_regenerate(context) is False