From 432702b9eaf78c01fb8a82d96bd0caa15c5b1f1f Mon Sep 17 00:00:00 2001 From: Goran Dodig Date: Wed, 14 Oct 2026 11:57:55 +0200 Subject: [PATCH] fix: name the cause when a render stops on failing unit tests When a functionality's unit tests still failed after its last render attempt, the render stopped with "ERROR codeplain: None" in the log, and the TUI got a dict it could not display. The stop now reads "Functionality 1: unit tests still fail after 2 render attempts, 20 fix rounds each. Last unit tests output: " in both the log and the TUI. ExitWithError resolves the stop reason once, and every other stop path logs its message instead of a raw payload. --- module_renderer.py | 6 +- render_machine/actions/exit_with_error.py | 15 +- .../actions/render_functional_requirement.py | 13 +- render_machine/code_renderer.py | 3 - render_machine/render_context.py | 3 +- render_machine/render_types.py | 20 +- tests/test_render_stop_reason.py | 210 ++++++++++++++++++ 7 files changed, 240 insertions(+), 30 deletions(-) create mode 100644 tests/test_render_stop_reason.py diff --git a/module_renderer.py b/module_renderer.py index 4ca655a2..20280fb6 100644 --- a/module_renderer.py +++ b/module_renderer.py @@ -134,10 +134,8 @@ def _render_module( code_renderer.run() if code_renderer.render_context.state == States.RENDER_FAILED.value: - error_message = RenderError.get_display_message( - code_renderer.render_context.previous_action_payload, - fallback_message=code_renderer.render_context.last_error_message, - ) + # ExitWithError resolved and logged the stop reason; the TUI shows the same text. + error_message = code_renderer.render_context.last_error_message or RenderError.get_display_message(None) code_renderer.render_context.event_bus.publish(RenderFailed(error_message=error_message)) return False, True diff --git a/render_machine/actions/exit_with_error.py b/render_machine/actions/exit_with_error.py index d49b956b..3ecbf92d 100644 --- a/render_machine/actions/exit_with_error.py +++ b/render_machine/actions/exit_with_error.py @@ -10,7 +10,13 @@ class ExitWithError(BaseAction): SUCCESSFUL_OUTCOME = "error_handled" def execute(self, render_context: RenderContext, previous_action_payload: Any | None): - console.error(previous_action_payload) + # The stop reason is the error payload of the failed action, or the message stored when the + # error was dispatched from a state callback. The log and the TUI both show this text. + error_message = RenderError.get_display_message( + previous_action_payload, fallback_message=render_context.last_error_message + ) + render_context.last_error_message = error_message + console.error(error_message) render_context.codeplain_api.fail_functional_requirement( render_context.frid_context.frid, @@ -27,9 +33,4 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | if render_context.run_state.render_id is not None: console.info(f"Render ID: {render_context.run_state.render_id}") - return ( - self.SUCCESSFUL_OUTCOME, - RenderError.encode( - message=render_context.last_error_message or "Unknown error", - ).to_payload(), - ) + return self.SUCCESSFUL_OUTCOME, RenderError.encode(message=error_message).to_payload() diff --git a/render_machine/actions/render_functional_requirement.py b/render_machine/actions/render_functional_requirement.py index 07fa924e..128e115c 100644 --- a/render_machine/actions/render_functional_requirement.py +++ b/render_machine/actions/render_functional_requirement.py @@ -7,7 +7,7 @@ from plain2code_exceptions import FunctionalRequirementTooComplex from render_machine.actions.base_action import BaseAction from render_machine.implementation_code_helpers import ImplementationCodeHelpers -from render_machine.render_context import RenderContext +from render_machine.render_context import MAX_UNITTEST_FIX_ATTEMPTS, RenderContext from render_machine.render_types import RenderError MAX_CODE_GENERATION_RETRIES = 2 @@ -20,9 +20,16 @@ class RenderFunctionalRequirement(BaseAction): def execute(self, render_context: RenderContext, _previous_action_payload: Any | None): if render_context.frid_context.functional_requirement_render_attempts >= MAX_CODE_GENERATION_RETRIES: - error_msg = f"Unittests could not be fixed after rendering the functionality {render_context.frid_context.frid} for the {MAX_CODE_GENERATION_RETRIES} times." + # A render attempt is only restarted once its unit tests used up every fix round. + error_msg = ( + f"Functionality {render_context.frid_context.frid}: unit tests still fail after " + f"{MAX_CODE_GENERATION_RETRIES} render attempts, {MAX_UNITTEST_FIX_ATTEMPTS} fix rounds each." + ) + last_output_path = render_context.script_execution_history.latest_unit_test_output_path + if last_output_path: + error_msg += f" Last unit tests output: {last_output_path}" render_context.last_error_message = error_msg - return self.ITERATION_LIMIT_EXCEEDED_OUTCOME, None + return self.ITERATION_LIMIT_EXCEEDED_OUTCOME, RenderError.encode(message=error_msg).to_payload() render_context.frid_context.functional_requirement_render_attempts += 1 diff --git a/render_machine/code_renderer.py b/render_machine/code_renderer.py index cb8de1a3..f940f664 100644 --- a/render_machine/code_renderer.py +++ b/render_machine/code_renderer.py @@ -67,14 +67,11 @@ def run(self): self.render_context.run_state.current_render_state = self.render_context.state self.render_context.script_execution_history.should_update_script_outputs = False - self.render_context.previous_action_payload = previous_action_payload - outcome, previous_action_payload = self.action_map[self.render_context.state].execute( self.render_context, previous_action_payload ) if self.render_context.state == States.RENDER_FAILED.value: - self.render_context.last_error_message = previous_action_payload self.render_context.event_bus.publish(RenderModuleFailed(module_name=self.render_context.module_name)) break diff --git a/render_machine/render_context.py b/render_machine/render_context.py index 75652953..71cfce0c 100644 --- a/render_machine/render_context.py +++ b/render_machine/render_context.py @@ -112,12 +112,11 @@ def set_machine(self, machine): self.machine = machine def dispatch_error(self, error_message: str): - """Log error, store it, and dispatch HANDLE_ERROR trigger. + """Store the error and dispatch HANDLE_ERROR trigger. ExitWithError logs it and the TUI displays it. Args: error_message: The error message to log and display to the user. """ - console.error(error_message) self.last_error_message = error_message self.machine.dispatch(triggers.HANDLE_ERROR) diff --git a/render_machine/render_types.py b/render_machine/render_types.py index 75a2f9b6..eb121412 100644 --- a/render_machine/render_types.py +++ b/render_machine/render_types.py @@ -214,17 +214,15 @@ def from_payload(cls, payload: Any) -> "RenderError | None": """Decode error from action payload. Expects standardized format: {"error": {"message": ..., "type": ..., "details": ...}} + Any other payload is not an error (e.g. the payload of the action that ran before an error was + dispatched from a state callback) and decodes to None. """ - if payload is None: + if not isinstance(payload, dict) or "error" not in payload: return None - if isinstance(payload, dict) and "error" in payload: - error_data = payload["error"] - return cls( - message=error_data.get("message", "Unknown error"), - error_type=error_data.get("type"), - details=error_data.get("details"), - ) - - # Unexpected format - log and return generic error - return cls(message=f"Unexpected error format: {type(payload).__name__}") + error_data = payload["error"] + return cls( + message=error_data.get("message", "Unknown error"), + error_type=error_data.get("type"), + details=error_data.get("details"), + ) diff --git a/tests/test_render_stop_reason.py b/tests/test_render_stop_reason.py new file mode 100644 index 00000000..c0e42619 --- /dev/null +++ b/tests/test_render_stop_reason.py @@ -0,0 +1,210 @@ +"""Tests for the reason a render gives when it stops on an error. + +When a render stops, the reason is logged and shown in the TUI. These tests drive the +real render state machine through the last failing unit-test run of a functionality +and check that the stop reason names the cause, instead of the literal "None". +""" + +import logging +import os +import stat +import tempfile +import threading +from types import SimpleNamespace + +import pytest + +import plain2code_logger +from event_bus import EventBus +from memory_management import MemoryManager +from plain2code_state import RunState +from plain_modules import PlainModule +from render_machine.actions.exit_with_error import ExitWithError +from render_machine.actions.render_functional_requirement import MAX_CODE_GENERATION_RETRIES +from render_machine.code_renderer import CodeRenderer +from render_machine.render_context import ( + MAX_FUNCTIONAL_REQUIREMENT_RENDER_ATTEMPTS_FAILED_UNIT_DURING_CONFORMANCE_TESTS, + MAX_UNITTEST_FIX_ATTEMPTS, + RenderContext, +) +from render_machine.render_types import RenderError, UnitTestsRunningContext +from render_machine.states import States + + +class FakeCodeplainAPI: + def __init__(self): + self.failed_frids = [] + + def fail_functional_requirement(self, frid, module_name, run_state): + self.failed_frids.append(frid) + + +@pytest.fixture +def workspace(): + with tempfile.TemporaryDirectory() as folder: + build_folder = os.path.join(folder, "plain_modules") + os.makedirs(build_folder) + + unittests_script = os.path.join(folder, "run_unittests.sh") + with open(unittests_script, "w", encoding="utf-8") as script: + script.write("#!/bin/sh\necho '1 failed, 0 passed'\nexit 1\n") + os.chmod(unittests_script, os.stat(unittests_script).st_mode | stat.S_IEXEC) + + yield build_folder, unittests_script + + +def make_render_context(api, build_folder, unittests_script, fixtures_dir): + plain_module = PlainModule("pr_solo.plain", build_folder, [fixtures_dir]) + os.makedirs(plain_module.module_build_folder, exist_ok=True) + return RenderContext( + api, + MemoryManager(api, plain_module.module_memory_folder), + plain_module, + build_folder=plain_module.module_build_folder, + build_dest="dist", + conformance_tests_dest="dist_tests", + unittests_script=unittests_script, + conformance_tests_script=None, + prepare_environment_script=None, + copy_build=False, + copy_conformance_tests=False, + render_range=None, + render_conformance_tests=False, + base_folder=fixtures_dir, + run_state=RunState(spec_filename="pr_solo.plain"), + event_bus=EventBus(), + enter_pause_event=threading.Event(), + ) + + +def nested_state(*states: States) -> str: + return "_".join(state.value for state in states) + + +def error_log_messages(caplog) -> list[str]: + return [ + record.getMessage() + for record in caplog.records + if record.name == plain2code_logger.LOGGER_NAME and record.levelno == logging.ERROR + ] + + +@pytest.fixture +def fixtures_dir(get_test_data_path): + return get_test_data_path("data/partial_rendering") + + +def test_unit_tests_failing_after_the_last_attempt_name_the_cause(workspace, fixtures_dir, caplog): + build_folder, unittests_script = workspace + api = FakeCodeplainAPI() + render_context = make_render_context(api, build_folder, unittests_script, fixtures_dir) + code_renderer = CodeRenderer(render_context) + + # The last render attempt of functionality 1, after its last unit-test fix round. + render_context.start_implementing_frid() + render_context.frid_context.functional_requirement_render_attempts = MAX_CODE_GENERATION_RETRIES + render_context.unit_tests_running_context = UnitTestsRunningContext(fix_attempts=MAX_UNITTEST_FIX_ATTEMPTS) + code_renderer.machine.set_state( + nested_state(States.IMPLEMENTING_FRID, States.PROCESSING_UNIT_TESTS, States.UNIT_TESTS_READY) + ) + + with caplog.at_level(logging.DEBUG, logger=plain2code_logger.LOGGER_NAME): + code_renderer.run() + + assert render_context.state == States.RENDER_FAILED.value + assert api.failed_frids == ["1"] + + last_output_path = render_context.script_execution_history.latest_unit_test_output_path + assert last_output_path is not None and os.path.exists(last_output_path) + + expected_message = ( + f"Functionality 1: unit tests still fail after {MAX_CODE_GENERATION_RETRIES} render attempts, " + f"{MAX_UNITTEST_FIX_ATTEMPTS} fix rounds each. Last unit tests output: {last_output_path}" + ) + assert error_log_messages(caplog) == [expected_message] + # The TUI shows the same stop reason as the log. + assert render_context.last_error_message == expected_message + + +def test_unit_tests_failing_again_during_conformance_tests_name_the_cause_once(workspace, fixtures_dir, caplog): + build_folder, unittests_script = workspace + api = FakeCodeplainAPI() + render_context = make_render_context(api, build_folder, unittests_script, fixtures_dir) + code_renderer = CodeRenderer(render_context) + + # Unit tests broken by a conformance fix, failing their last fix round for the last allowed time. + render_context.start_implementing_frid() + render_context.start_conformance_tests_processing() + render_context.functional_requirements_render_attempts_failed_unit_during_conformance_tests = ( + MAX_FUNCTIONAL_REQUIREMENT_RENDER_ATTEMPTS_FAILED_UNIT_DURING_CONFORMANCE_TESTS - 1 + ) + render_context.unit_tests_running_context = UnitTestsRunningContext(fix_attempts=MAX_UNITTEST_FIX_ATTEMPTS) + code_renderer.machine.set_state( + nested_state( + States.IMPLEMENTING_FRID, + States.PROCESSING_CONFORMANCE_TESTS, + States.PROCESSING_UNIT_TESTS, + States.UNIT_TESTS_READY, + ) + ) + + with caplog.at_level(logging.DEBUG, logger=plain2code_logger.LOGGER_NAME): + code_renderer.run() + + assert render_context.state == States.RENDER_FAILED.value + error_messages = error_log_messages(caplog) + assert len(error_messages) == 1 + assert error_messages[0].startswith("Failed to adjust the unit tests") + assert "functionality 1" in error_messages[0] + # The TUI shows the same stop reason as the log, not the unit tests output of the payload. + assert render_context.last_error_message == error_messages[0] + + +class TestExitWithErrorMessage: + @staticmethod + def make_context(last_error_message=None): + render_context = RenderContext.__new__(RenderContext) + render_context.codeplain_api = FakeCodeplainAPI() + render_context.frid_context = SimpleNamespace(frid="1") + render_context.module_name = "mod" + render_context.run_state = RunState(spec_filename="mod.plain") + render_context.last_error_message = last_error_message + return render_context + + def run_exit(self, render_context, payload, caplog): + with caplog.at_level(logging.DEBUG, logger=plain2code_logger.LOGGER_NAME): + ExitWithError().execute(render_context, payload) + return error_log_messages(caplog) + + def test_logs_the_message_of_an_error_payload(self, caplog): + render_context = self.make_context() + payload = RenderError.encode( + message="Unit tests script failed due to problems in the environment setup.", + error_type="ENVIRONMENT_ERROR", + script="run_unittests.sh", + ).to_payload() + + messages = self.run_exit(render_context, payload, caplog) + + expected = ( + "Unit tests script failed due to problems in the environment setup.\n\nDetails:\n Script: run_unittests.sh" + ) + assert messages == [expected] + assert render_context.last_error_message == expected + + def test_logs_the_stored_message_when_the_payload_is_not_an_error(self, caplog): + render_context = self.make_context(last_error_message="Conformance tests could not be fixed.") + + messages = self.run_exit(render_context, {"previous_unittests_issue": "1 failed"}, caplog) + + assert messages == ["Conformance tests could not be fixed."] + assert render_context.last_error_message == "Conformance tests could not be fixed." + + def test_never_logs_none(self, caplog): + render_context = self.make_context() + + messages = self.run_exit(render_context, None, caplog) + + assert len(messages) == 1 + assert "None" not in messages[0] + assert render_context.last_error_message == messages[0]