diff --git a/codeplain_REST_api.py b/codeplain_REST_api.py index a1a3a044..01d609dc 100644 --- a/codeplain_REST_api.py +++ b/codeplain_REST_api.py @@ -253,38 +253,6 @@ def render_functional_requirement( return self.post_request(endpoint_url, headers, payload, run_state) - def fix_unittests_issue( - self, - frid, - plain_source_tree, - linked_resources, - existing_files_content, - module_name: str, - required_modules, - unittests_issue, - run_state: RunState, - conformance_tests_fixes: list[dict] | None = None, - ): - endpoint_url = f"{self.api_url}/fix_unittests_issue" - headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"} - - payload = { - "frid": frid, - "plain_source_tree": plain_source_tree, - "linked_resources": linked_resources, - "existing_files_content": existing_files_content, - "module_name": module_name, - "required_modules": required_modules, - "unittests_issue": unittests_issue, - "unittest_batch_id": run_state.unittest_batch_id, - } - # Implementation code changes made by the conformance tests fixer right before this unit tests run; - # only sent when unit tests are processed inside the conformance tests phase. - if conformance_tests_fixes is not None: - payload["conformance_tests_fixes"] = conformance_tests_fixes - - return self.post_request(endpoint_url, headers, payload, run_state) - def distill_conformance_test_memory( self, frid, @@ -558,3 +526,29 @@ def summarize_finished_conformance_tests( } return self.post_request(endpoint_url, headers, payload, run_state) + + def agent_start(self, task_type: str, task_params: dict, frid: str, module_name: str, run_state: RunState): + """Start a server-side agent session; returns the first turn (tool calls or completion).""" + endpoint_url = f"{self.api_url}/agent/start" + headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"} + payload = { + "task_type": task_type, + "task_params": task_params, + "frid": frid, + "module_name": module_name, + } + return self.post_request(endpoint_url, headers, payload, run_state) + + def agent_continue( + self, session_id: str, tool_results: list[dict], frid: str, module_name: str, run_state: RunState + ): + """Feed tool results into an agent session and run its next turn.""" + endpoint_url = f"{self.api_url}/agent/continue" + headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"} + payload = { + "session_id": session_id, + "tool_results": tool_results, + "frid": frid, + "module_name": module_name, + } + return self.post_request(endpoint_url, headers, payload, run_state) diff --git a/render_machine/actions/fix_unit_tests.py b/render_machine/actions/fix_unit_tests.py index 50a54b6a..4575b9e2 100644 --- a/render_machine/actions/fix_unit_tests.py +++ b/render_machine/actions/fix_unit_tests.py @@ -1,79 +1,256 @@ +import os +from http import HTTPStatus from typing import Any +import requests + import file_utils -import render_machine.render_utils as render_utils +import plain_spec from plain2code_console import console from plain2code_exceptions import InternalClientError +from render_machine import agent_tools from render_machine.actions.base_action import BaseAction -from render_machine.implementation_code_helpers import ImplementationCodeHelpers from render_machine.render_context import RenderContext -MAX_ISSUE_LENGTH = 10000 +TASK_TYPE = "fix_unit_tests" +SUBMIT_FIX_TOOL = "submit_fix" +# Upper bound on LLM turns spent on one fix attempt; the server bounds the whole session. +MAX_AGENT_TURNS_PER_ATTEMPT = 40 +# Answer to a submit_fix whose fix was accepted in an earlier unit-test loop of the conformance phase. +ACCEPTED_THEN_CHANGED_MESSAGE = ( + "Your fix was accepted: the unit tests passed. Afterwards the implementation code was changed to fix the " + "conformance tests{see_below}, and the unit tests now fail again. Files may have changed since you last read " + "them." +) +# Seeding the first turn: the build folder's file list and the files changed for the FRID. +MAX_FILE_TREE_ENTRIES = 500 +MAX_RELEVANT_FILES_CHARS = 60_000 class FixUnitTests(BaseAction): + """Fix failing unit tests with a server-side agent session that spans the fix attempts. + + The first failure starts a session; the agent then drives read/grep/edit/run tool calls + (executed here) until it calls submit_fix. The state machine re-runs the unit tests and, + if they still fail, the next execution of this action answers that submit_fix call with + the new failure output inside the same session, so earlier attempts stay in context. + + During the conformance phase the session also spans unit-test loops: after the conformance + tests fixer changes implementation code, the unit tests fail again and the same session is + continued, told what the conformance tests fixer changed (which it must preserve). + """ + SUCCESSFUL_OUTCOME = "unit_tests_fix_generated" def execute(self, render_context: RenderContext, previous_action_payload: Any | None): - if not previous_action_payload.get("previous_unittests_issue"): + if not previous_action_payload or not previous_action_payload.get("previous_unittests_issue"): raise InternalClientError( "Internal client error: Previous action payload does not contain previous unit tests issue." ) - previous_unittests_issue = previous_action_payload["previous_unittests_issue"] - - if previous_unittests_issue and len(previous_unittests_issue) > MAX_ISSUE_LENGTH: - console.debug( - f"Unit tests issue text is too long and will be smartly truncated to {MAX_ISSUE_LENGTH} characters." - ) - - existing_files, existing_files_content = ImplementationCodeHelpers.fetch_existing_files( - render_context.build_folder - ) - - render_utils.print_inputs(render_context, existing_files_content, "Files sent as input to unit tests fixing:") - + unittests_issue = previous_action_payload["previous_unittests_issue"] + context = render_context.unit_tests_running_context + session = render_context.unit_tests_agent_session + api = render_context.codeplain_api + frid, module_name = render_context.frid_context.frid, render_context.module_name + changed_files_before = set(context.changed_files) + log_path = render_context.script_execution_history.latest_unit_test_output_path + if log_path: + agent_tools.register_log_path(log_path, render_context) conformance_tests_fixes = self._get_conformance_tests_fixes(render_context) - if conformance_tests_fixes: + new_conformance_tests_fixes = conformance_tests_fixes[session.conformance_fixes_handed_off :] + if new_conformance_tests_fixes: console.info( - f"Unit tests are fixed while preserving {len(conformance_tests_fixes)} implementation code change(s) " - "made to fix the conformance tests." + f"Unit tests are fixed while preserving {len(new_conformance_tests_fixes)} implementation code " + "change(s) made to fix the conformance tests." ) - response_files = render_context.codeplain_api.fix_unittests_issue( - render_context.frid_context.frid, - render_context.plain_source_tree, - render_context.frid_context.linked_resources, - existing_files_content, - render_context.module_name, - render_context.get_required_modules_functionalities(), - previous_unittests_issue, - run_state=render_context.run_state, - conformance_tests_fixes=conformance_tests_fixes, - ) - - _, changed_files = file_utils.update_build_folder_with_rendered_files( - render_context.build_folder, existing_files, response_files - ) + response = None + if session.session_id is not None: + if context.agent_used_in_this_loop: + console.info(f"Continuing agent session {session.session_id} with the new unit tests failure.") + output = "The fix was applied, but the unit tests still fail." + else: + # The session's last fix was accepted in an earlier unit-test loop of this conformance + # phase; since then the conformance tests fixer changed the code and the tests fail again. + console.info( + f"Continuing agent session {session.session_id}: the unit tests fail again after the " + "implementation was changed to fix the conformance tests." + ) + output = ACCEPTED_THEN_CHANGED_MESSAGE.format( + see_below=" (see the Conformance Tests Fix below)" if new_conformance_tests_fixes else "" + ) + # Files changed outside the session, so earlier read results are stale. + context.tool_result_cache.clear() + submit_result: dict = { + "call_id": session.pending_submit_call_id, + "output": output + agent_tools.full_log_pointer(log_path), + "test_output": unittests_issue, + } + if new_conformance_tests_fixes: + submit_result["conformance_tests_fixes"] = new_conformance_tests_fixes + tool_results = session.pending_tool_results + [submit_result] + session.pending_tool_results, session.pending_submit_call_id = [], None + response = self._continue_session(render_context, session.session_id, tool_results) + if response is None: + console.info("Starting an agent session to fix the unit tests.") + # Cached read results point at earlier turns, which a new session does not have. + context.tool_result_cache.clear() + response = api.agent_start( + TASK_TYPE, + self._build_task_params(render_context, unittests_issue, conformance_tests_fixes), + frid, + module_name, + render_context.run_state, + ) + session.session_id = response["session_id"] + assert session.session_id is not None + session.conformance_fixes_handed_off = len(conformance_tests_fixes) + context.agent_used_in_this_loop = True - render_context.unit_tests_running_context.changed_files.update(changed_files) + submitted = False + turns = 0 + while response.get("status") == "tool_calls" and turns < MAX_AGENT_TURNS_PER_ATTEMPT: + turns += 1 + calls = response["calls"] + submit_call = next((call for call in calls if call["name"] == SUBMIT_FIX_TOOL), None) + tool_results = agent_tools.execute_calls( + [call for call in calls if call is not submit_call], render_context + ) + if submit_call is not None: + session.pending_submit_call_id = submit_call["id"] + session.pending_tool_results = tool_results + submitted = True + console.info(f"Agent submitted a fix: {submit_call['args'].get('changes_made', '')}") + break + response = api.agent_continue(session.session_id, tool_results, frid, module_name, render_context.run_state) - console.print_files("Files fixed:", render_context.build_folder, response_files, style=console.OUTPUT_STYLE) + if not submitted: + # The session ended without a submission (finished in text, failed, or used up this + # attempt's turn budget) — a fresh session is started if the tests still fail. + status = response.get("status") + if status == "failed": + console.warning(f"Agent session failed: {response.get('error', 'unknown error')}") + elif status == "tool_calls": + console.warning(f"Agent used {MAX_AGENT_TURNS_PER_ATTEMPT} turns without submitting a fix.") + session.reset() + console.print_files( + "Files changed while fixing unit tests:", + render_context.build_folder, + {path: "" for path in sorted(context.changed_files - changed_files_before)}, + style=console.OUTPUT_STYLE, + ) return self.SUCCESSFUL_OUTCOME, None @staticmethod - def _get_conformance_tests_fixes(render_context: RenderContext) -> list[dict] | None: - """Implementation code changes the conformance tests fixer made before these unit tests were run. - - Only present when unit tests are processed inside the conformance tests phase - the implementation - and refactoring unit test passes have no conformance tests running context. - """ + def _get_conformance_tests_fixes(render_context: RenderContext) -> list[dict]: + """Implementation code changes the conformance tests fixer made during this conformance phase, oldest + first. Empty outside the conformance phase (the implementation and refactoring unit-test loops).""" conformance_tests_running_context = getattr(render_context, "conformance_tests_running_context", None) if conformance_tests_running_context is None: - return None + return [] + return list(getattr(conformance_tests_running_context, "implementation_code_fixes", None) or []) - implementation_code_fixes = getattr(conformance_tests_running_context, "implementation_code_fixes", None) - if not implementation_code_fixes: + @staticmethod + def _continue_session(render_context: RenderContext, session_id: str, tool_results: list[dict]) -> dict | None: + """Continue the session; None if the server no longer has it (expired), so a new one is started.""" + try: + return render_context.codeplain_api.agent_continue( + session_id, + tool_results, + render_context.frid_context.frid, + render_context.module_name, + render_context.run_state, + ) + except requests.exceptions.HTTPError as e: + if e.response is None or e.response.status_code != HTTPStatus.NOT_FOUND: + raise + console.warning(f"Agent session {session_id} has expired; starting a new one.") + render_context.unit_tests_agent_session.reset() return None - return list(implementation_code_fixes) + @staticmethod + def _build_task_params( + render_context: RenderContext, unittests_issue: str, conformance_tests_fixes: list[dict] + ) -> dict: + frid = render_context.frid_context.frid + specifications, _ = plain_spec.get_specifications_for_frid(render_context.plain_source_tree, frid) + context = render_context.unit_tests_running_context + # The files the conformance tests fixes changed are always seeded, so the agent sees what to preserve. + conformance_fix_files = {name for fix in conformance_tests_fixes for name in (fix or {}).get("code_diff") or {}} + task_params = { + "definitions": "\n".join(specifications.get(plain_spec.DEFINITIONS, [])), + "non_functional_requirements": "\n".join(specifications.get(plain_spec.NON_FUNCTIONAL_REQUIREMENTS, [])), + "functional_requirements": FixUnitTests._functional_requirements_section(render_context, specifications), + "linked_resources": render_context.frid_context.linked_resources, + "build_folder": render_context.build_folder, + "module_name": render_context.module_name, + "unittests_script_content": FixUnitTests._read_script(render_context.unittests_script), + "unittests_issue": unittests_issue, + "unittests_log_path": render_context.script_execution_history.latest_unit_test_output_path, + "file_tree": FixUnitTests._file_tree(render_context.build_folder), + "relevant_files": FixUnitTests._relevant_files( + render_context.build_folder, + render_context.frid_context.changed_files | context.changed_files | conformance_fix_files, + ), + } + if conformance_tests_fixes: + task_params["conformance_tests_fixes"] = conformance_tests_fixes + session = render_context.unit_tests_agent_session + if session.previous_session_id: + task_params["previous_session_id"] = session.previous_session_id + return task_params + + @staticmethod + def _file_tree(build_folder: str) -> str: + paths = [] + for root, dirs, files in os.walk(build_folder): + dirs[:] = sorted(d for d in dirs if d not in agent_tools.GREP_EXCLUDED_DIRS and not d.startswith(".")) + paths.extend(os.path.relpath(os.path.join(root, name), build_folder) for name in sorted(files)) + if len(paths) > MAX_FILE_TREE_ENTRIES: + return "\n".join(paths[:MAX_FILE_TREE_ENTRIES]) + "\n... [more files not listed]" + return "\n".join(paths) + + @staticmethod + def _relevant_files(build_folder: str, file_names: set[str]) -> dict[str, str]: + """Contents of the given build-relative files, smallest first, within MAX_RELEVANT_FILES_CHARS.""" + contents = {} + for name in file_names: + full_path = os.path.join(build_folder, name) + if os.path.isfile(full_path): + with open(full_path, "r", encoding="utf-8", errors="replace") as f: + contents[name] = f.read() + relevant, total = {}, 0 + for name in sorted(contents, key=lambda n: (len(contents[n]), n)): + if total + len(contents[name]) > MAX_RELEVANT_FILES_CHARS: + break + relevant[name] = contents[name] + total += len(contents[name]) + return dict(sorted(relevant.items())) + + @staticmethod + def _functional_requirements_section(render_context: RenderContext, specifications: dict) -> str: + sections = [] + for module_name, functionalities in render_context.get_required_modules_functionalities().items(): + sections.append( + f"### Module: {module_name} (Already Implemented, for context)\n" + "\n".join(functionalities) + ) + current = specifications.get(plain_spec.FUNCTIONAL_REQUIREMENTS, []) + if len(current) > 1: + sections.append( + f"### Module: {render_context.module_name} (Already Implemented, for context)\n" + + "\n".join(current[:-1]) + ) + if current: + sections.append(f"### Module: {render_context.module_name} (Currently Being Implemented)\n{current[-1]}") + return "\n\n".join(sections) + + @staticmethod + def _read_script(script: str | None) -> str: + if not script: + return "" + try: + with open(file_utils.add_current_path_if_no_path(script), "r", encoding="utf-8") as f: + return f.read() + except OSError: + return "" diff --git a/render_machine/actions/run_unit_tests.py b/render_machine/actions/run_unit_tests.py index 79e29dbc..93b0e828 100644 --- a/render_machine/actions/run_unit_tests.py +++ b/render_machine/actions/run_unit_tests.py @@ -16,6 +16,15 @@ class RunUnitTests(BaseAction): UNRECOVERABLE_ERROR_OUTCOME = "unrecoverable_error_occurred" def execute(self, render_context: RenderContext, _previous_action_payload: Any | None): + context = render_context.unit_tests_running_context + if context.verified_passing: + # The fixing agent's own run passed and no file changed since; running again is redundant. + context.verified_passing = False + console.info("Unit tests already passed in the fixing agent's last run; not running them again.") + render_context.script_execution_history.latest_unit_test_output_path = context.verified_passing_log_path + render_context.script_execution_history.should_update_script_outputs = True + return self.SUCCESSFUL_OUTCOME, None + unittests_script = os.path.normpath(render_context.unittests_script) console.info( diff --git a/render_machine/agent_tools.py b/render_machine/agent_tools.py new file mode 100644 index 00000000..14c7309c --- /dev/null +++ b/render_machine/agent_tools.py @@ -0,0 +1,270 @@ +"""Client-side implementations of the tools a server-side agent can call. + +The server declares the tools to the LLM (codeplain-api: src/agent/tools.py) and forwards +the model's calls; this module executes them against the local build folder and returns +plain-text results. Relative paths resolve against the build folder. Reads are allowed only in the +build folder and the full test logs the agent was pointed to; writes only inside the build folder. +""" + +import glob +import json +import os +import subprocess +import tempfile +from typing import Callable + +from plain2code_console import console +from render_machine import render_utils +from render_machine.render_context import RenderContext + +DEFAULT_READ_LIMIT = 200 +MAX_LINE_CHARS = 10_000 +MAX_OUTPUT_CHARS = 30_000 +EDIT_SNIPPET_CONTEXT_LINES = 3 +MAX_EDIT_SNIPPET_LINES = 60 +MAX_GREP_CONTEXT_LINES = 20 +# Tools without side effects; repeating one while no file changed returns a pointer instead. +READ_ONLY_TOOLS = ("read_file", "grep", "ls_files") +GREP_EXCLUDED_DIRS = (".git", "__pycache__", "node_modules", ".venv", "target", "dist", "build") + + +def _build_folder(render_context: RenderContext) -> str: + return os.path.normpath(os.path.abspath(render_context.build_folder)) + + +def _resolve(file_path: str, render_context: RenderContext) -> str: + if os.path.isabs(file_path): + return os.path.normpath(file_path) + return os.path.normpath(os.path.join(_build_folder(render_context), file_path)) + + +def _within(path: str, folder: str) -> bool: + return path == folder or path.startswith(folder + os.sep) + + +def _readable(path: str, render_context: RenderContext) -> bool: + return ( + _within(path, _build_folder(render_context)) + or path in render_context.unit_tests_agent_session.readable_log_paths + ) + + +def register_log_path(log_path: str, render_context: RenderContext) -> None: + """Allow read_file/grep on a full test log the agent is pointed to.""" + render_context.unit_tests_agent_session.readable_log_paths.add(os.path.normpath(os.path.abspath(log_path))) + + +def _writable(path: str, render_context: RenderContext) -> bool: + return _within(path, _build_folder(render_context)) + + +def _bound(text: str) -> str: + """Cap very long lines and the total size so one tool result cannot flood the context.""" + lines = [ + line if len(line) <= MAX_LINE_CHARS else line[:MAX_LINE_CHARS] + "... [line truncated]" + for line in text.split("\n") + ] + text = "\n".join(lines) + if len(text) > MAX_OUTPUT_CHARS: + head, tail = int(MAX_OUTPUT_CHARS * 0.6), int(MAX_OUTPUT_CHARS * 0.4) + text = text[:head] + f"\n\n... [truncated {len(text) - head - tail:,} chars] ...\n\n" + text[-tail:] + return text + + +def _track_change(full_path: str, render_context: RenderContext) -> None: + context = render_context.unit_tests_running_context + context.changed_files.add(os.path.relpath(full_path, _build_folder(render_context))) + context.verified_passing, context.verified_passing_log_path = False, None + context.tool_result_cache.clear() + + +def read_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + if not _readable(full_path, render_context): + return f"Error: read access denied for '{full_path}' (readable: build folder and test logs)." + if not os.path.isfile(full_path): + return f"Error: file not found: '{full_path}'." + with open(full_path, "r", encoding="utf-8", errors="replace") as f: + lines = f.read().split("\n") + offset = max(int(args.get("offset") or 1), 1) + limit = int(args.get("limit") or DEFAULT_READ_LIMIT) + selected = lines[offset - 1 : offset - 1 + limit] + if not selected: + return f"Error: offset {offset} is past the end of the file ({len(lines)} lines)." + numbered = "\n".join(f"{offset + i}: {line}" for i, line in enumerate(selected)) + last = offset - 1 + len(selected) + note = ( + f"\n[showing lines {offset}-{last} of {len(lines)}; use offset={last + 1} to continue]" + if last < len(lines) + else "" + ) + return _bound(numbered) + note + + +def grep(args: dict, render_context: RenderContext) -> str: + pattern = args.get("pattern", "") + if not pattern: + return "Error: pattern is required." + target = _resolve(args.get("file_path") or ".", render_context) + if not _readable(target, render_context): + return f"Error: read access denied for '{target}'." + if not os.path.exists(target): + return f"Error: path not found: '{target}'." + # Run from the build folder so matches inside it come back as build-relative paths, which + # is the form the other tools accept; a test log outside it is passed by absolute path. + cwd = _build_folder(render_context) + options = [f"--exclude-dir={d}" for d in GREP_EXCLUDED_DIRS] + context_lines = min(max(int(args.get("context_lines") or 0), 0), MAX_GREP_CONTEXT_LINES) + if context_lines: + options.append(f"-C{context_lines}") + if args.get("include"): + options.append(f"--include={args['include']}") + command = ["grep", "-rnI", *options, "-e", pattern, "--"] + command.append(os.path.relpath(target, cwd) if _within(target, cwd) else target) + result = subprocess.run(command, capture_output=True, text=True, cwd=cwd) + if result.returncode == 1: + return f"No matches for '{pattern}' in '{target}'." + if result.returncode != 0: + return f"Error: grep failed: {result.stderr.strip()}" + lines = [line[2:] if line.startswith("./") else line for line in result.stdout.rstrip("\n").split("\n")] + return _bound("\n".join(lines)) + + +def ls_files(args: dict, render_context: RenderContext) -> str: + target = _resolve(args.get("pattern") or ".", render_context) + if not _readable(target, render_context): + return f"Error: read access denied for '{target}'." + if os.path.isdir(target): + entries = sorted(os.listdir(target)) + listing = [entry + "/" if os.path.isdir(os.path.join(target, entry)) else entry for entry in entries] + return f"{target}:\n" + ("\n".join(listing) if listing else "(empty)") + matches = sorted(glob.glob(target, recursive=True)) + return _bound("\n".join(matches)) if matches else f"No files match '{target}'." + + +def edit_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + search, replace = args.get("search", ""), args.get("replace", "") + if not search: + return "Error: search is required." + if not _writable(full_path, render_context): + return f"Error: write access denied for '{full_path}' (writable: build folder only)." + if not os.path.isfile(full_path): + return f"Error: file not found: '{full_path}'. Use write_file to create new files." + with open(full_path, "r", encoding="utf-8") as f: + content = f.read() + occurrences = content.count(search) + if occurrences != 1: + return ( + f"Error: search text found {occurrences} times in '{full_path}'; it must appear exactly once. " + "Read the file and use a larger, unique snippet." + ) + with open(full_path, "w", encoding="utf-8") as f: + f.write(content.replace(search, replace, 1)) + _track_change(full_path, render_context) + return f"Edited '{full_path}'. The edited region now reads:\n" + _edit_snippet(content, search, replace) + + +def _edit_snippet(content: str, search: str, replace: str) -> str: + """Numbered lines of the replacement plus a little context, so the edit needs no re-read.""" + lines = content.replace(search, replace, 1).split("\n") + first = content[: content.index(search)].count("\n") + last = first + replace.count("\n") + start = max(first - EDIT_SNIPPET_CONTEXT_LINES, 0) + end = min(last + EDIT_SNIPPET_CONTEXT_LINES + 1, len(lines), start + MAX_EDIT_SNIPPET_LINES) + snippet = "\n".join(f"{start + i + 1}: {line}" for i, line in enumerate(lines[start:end])) + return _bound(snippet) + + +def write_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + if not _writable(full_path, render_context): + return f"Error: write access denied for '{full_path}' (writable: build folder only)." + os.makedirs(os.path.dirname(full_path), exist_ok=True) + with open(full_path, "w", encoding="utf-8") as f: + f.write(args.get("content", "")) + _track_change(full_path, render_context) + return f"Wrote '{full_path}'." + + +def delete_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + if not _writable(full_path, render_context): + return f"Error: write access denied for '{full_path}' (writable: build folder only)." + if not os.path.isfile(full_path): + return f"Error: file not found: '{full_path}'." + os.remove(full_path) + _track_change(full_path, render_context) + return f"Deleted '{full_path}'." + + +def full_log_pointer(log_file_path: str | None) -> str: + return f" Full log: {log_file_path} (search it with grep, passing that path as file_path)." if log_file_path else "" + + +def run_unit_tests(_args: dict, render_context: RenderContext) -> dict: + """Returns a result dict: a short header in `output` and the raw failure output in + `test_output`, which the server condenses (summarizing it when it is long).""" + exit_code, output, log_file_path = render_utils.execute_script( + os.path.normpath(render_context.unittests_script), + [render_context.build_folder], + "Unit Tests", + timeout=render_context.test_script_timeout, + stop_event=render_context.stop_event, + ) + context = render_context.unit_tests_running_context + if exit_code == 0: + context.verified_passing, context.verified_passing_log_path = True, log_file_path + return {"output": "All unit tests passed."} + if not log_file_path and output: + with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False, suffix=".unittest_output") as f: + f.write(output) + log_file_path = f.name + if log_file_path: + register_log_path(log_file_path, render_context) + return { + "output": f"Unit tests failed (exit code {exit_code}).{full_log_pointer(log_file_path)}", + "test_output": output, + } + + +TOOLS: dict[str, Callable[[dict, RenderContext], str | dict]] = { + "read_file": read_file, + "grep": grep, + "ls_files": ls_files, + "edit_file": edit_file, + "write_file": write_file, + "delete_file": delete_file, + "run_unit_tests": run_unit_tests, +} + + +def execute_calls(calls: list[dict], render_context: RenderContext) -> list[dict]: + """Execute the agent's tool calls in order; every call gets a result, errors included. + + A repeated read-only call (same tool and arguments, no file changed in between) is not + re-executed: its result is already in the conversation, so a short pointer is returned.""" + cache = render_context.unit_tests_running_context.tool_result_cache + results = [] + for call in calls: + tool = TOOLS.get(call["name"]) + cache_key = json.dumps([call["name"], call.get("args") or {}], sort_keys=True) + result: dict + if tool is None: + result = {"output": f"Error: unknown tool '{call['name']}'."} + elif call["name"] in READ_ONLY_TOOLS and cache_key in cache: + result = { + "output": "Same call as an earlier one and no file has changed since; " + "its result is unchanged (see the earlier result above)." + } + else: + try: + output = tool(call.get("args") or {}, render_context) + result = output if isinstance(output, dict) else {"output": output} + except Exception as e: + result = {"output": f"Error: tool '{call['name']}' failed: {type(e).__name__}: {e}"} + if call["name"] in READ_ONLY_TOOLS and not result["output"].startswith("Error"): + cache[cache_key] = result["output"] + console.debug(f"Agent tool {call['name']}({call.get('args')}) -> {result['output'][:200]!r}") + results.append({"call_id": call["id"], **result}) + return results diff --git a/render_machine/render_context.py b/render_machine/render_context.py index 75652953..62ba9331 100644 --- a/render_machine/render_context.py +++ b/render_machine/render_context.py @@ -19,6 +19,7 @@ FridContext, ScriptExecutionHistory, TestExecutionPhase, + UnitTestsAgentSession, UnitTestsRunningContext, ) @@ -188,6 +189,14 @@ def should_run_unit_tests(self) -> bool: def should_run_conformance_tests(self) -> bool: return self.conformance_tests_script is not None + @property + def unit_tests_agent_session(self) -> UnitTestsAgentSession: + """The agent session fixing the unit tests: per unit-test loop, except during the conformance + phase, where one session spans every loop (see UnitTestsAgentSession).""" + if self.conformance_tests_running_context is not None: + return self.conformance_tests_running_context.unit_tests_agent_session + return self.unit_tests_running_context.agent_session + def start_unittests_processing(self): self.unit_tests_running_context = UnitTestsRunningContext(fix_attempts=0) self.run_state.increment_unittest_batch_id() diff --git a/render_machine/render_types.py b/render_machine/render_types.py index 75a2f9b6..269e6bb9 100644 --- a/render_machine/render_types.py +++ b/render_machine/render_types.py @@ -48,10 +48,53 @@ class FridContext: refactoring_iteration: int = 0 +@dataclass +class UnitTestsAgentSession: + """Server-side agent session fixing a FRID's unit tests, and what it still has to be told. + + Owned by the unit-tests running context (one session per unit-test loop) in the implementation + and refactoring phases, and by the conformance tests running context during the conformance + phase, so there one session spans every unit-test loop - the agent then sees that the + conformance tests fixer keeps changing the code it adjusts (see RenderContext.unit_tests_agent_session). + """ + + session_id: Optional[str] = None + # The submit_fix call the agent ended its last attempt with, answered with the next test + # run's outcome, plus results of any tool calls made in the same turn as submit_fix. + pending_submit_call_id: Optional[str] = None + pending_tool_results: list[dict] = field(default_factory=list) + # Session abandoned without a submission (turn budget used up, LLM failure); the next session + # starts with a digest of what it tried. + previous_session_id: Optional[str] = None + # Full test logs the agent was pointed to; readable by read_file/grep although outside the + # build folder. + readable_log_paths: set[str] = field(default_factory=set) + # How many of the conformance tests fixes (ConformanceTestsRunningContext.implementation_code_fixes) + # the session has already been shown. + conformance_fixes_handed_off: int = 0 + + def reset(self) -> None: + """Drop the session (keeping its id as the previous one) so the next failure starts a new one.""" + self.previous_session_id = self.session_id + self.session_id, self.pending_submit_call_id, self.pending_tool_results = None, None, [] + self.conformance_fixes_handed_off = 0 + + @dataclass class UnitTestsRunningContext: fix_attempts: int changed_files: set[str] = field(default_factory=set) + # The agent session of this loop; used only outside the conformance phase. + agent_session: UnitTestsAgentSession = field(default_factory=UnitTestsAgentSession) + # Whether the agent already made a fix attempt in this loop. A session that is still open when + # a loop starts had its last fix accepted, which is what the agent is told. + agent_used_in_this_loop: bool = False + # Set when the agent's own run_unit_tests passed and no file changed since, so the harness + # can accept the fix without running the suite again. + verified_passing: bool = False + verified_passing_log_path: Optional[str] = None + # Results of read-only tool calls, keyed by call; cleared whenever a file changes. + tool_result_cache: dict[str, str] = field(default_factory=dict) class ConformanceTestsRunningContext: @@ -95,6 +138,8 @@ def __init__( # order. Each entry is {"hypothesis": str | None, "approach": str | None, "code_diff": {file: diff}}. # Handed to the unit tests fixer so it adjusts the unit tests instead of reverting these changes. self.implementation_code_fixes: list[dict] = [] + # The unit-test fixing agent session, shared by every unit-test loop of this conformance phase. + self.unit_tests_agent_session = UnitTestsAgentSession() def get_conformance_tests_json(self, module_name: str) -> dict: return self._conformance_tests_json[module_name] diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py new file mode 100644 index 00000000..e4e8ed4a --- /dev/null +++ b/tests/test_agent_tools.py @@ -0,0 +1,166 @@ +"""Tests for the client-side agent tool implementations.""" + +import os +from types import SimpleNamespace + +import pytest + +from render_machine import agent_tools +from render_machine.render_context import RenderContext +from render_machine.render_types import UnitTestsRunningContext + + +class FakeRenderContext(SimpleNamespace): + unit_tests_agent_session = RenderContext.unit_tests_agent_session + + +@pytest.fixture +def project(tmp_path, monkeypatch): + build = tmp_path / "plain_modules" / "m" + build.mkdir(parents=True) + (build / "app.py").write_text("def add(a, b):\n return a - b\n\n\ndef sub(a, b):\n return a - b\n") + (tmp_path / "outside.txt").write_text("outside\n") + monkeypatch.chdir(tmp_path) + render_context = FakeRenderContext( + build_folder=str(build), + unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), + conformance_tests_running_context=None, + unittests_script=None, + test_script_timeout=None, + stop_event=None, + ) + return SimpleNamespace(root=tmp_path, build=build, rc=render_context) + + +def test_read_file_resolves_relative_to_build_folder_with_paging(project): + out = agent_tools.read_file({"file_path": "app.py", "offset": 2, "limit": 1}, project.rc) + assert out.startswith("2: return a - b") + assert "use offset=3 to continue" in out + + +def test_read_is_confined_to_build_folder(project): + outside = str(project.root / "outside.txt") + assert agent_tools.read_file({"file_path": outside}, project.rc).startswith("Error: read access denied") + assert agent_tools.read_file({"file_path": "../../outside.txt"}, project.rc).startswith("Error: read access denied") + assert agent_tools.grep({"pattern": "outside", "file_path": outside}, project.rc).startswith("Error: read access") + assert agent_tools.ls_files({"pattern": str(project.root)}, project.rc).startswith("Error: read access denied") + assert agent_tools.read_file({"file_path": "/etc/hosts"}, project.rc).startswith("Error: read access denied") + assert agent_tools.read_file({"file_path": "missing.py"}, project.rc).startswith("Error: file not found") + + +def test_edit_file_requires_unique_match_and_tracks_change(project): + ambiguous = agent_tools.edit_file({"file_path": "app.py", "search": "return a - b", "replace": "x"}, project.rc) + assert "found 2 times" in ambiguous + assert project.rc.unit_tests_running_context.changed_files == set() + + ok = agent_tools.edit_file( + { + "file_path": "app.py", + "search": "def add(a, b):\n return a - b", + "replace": "def add(a, b):\n return a + b", + }, + project.rc, + ) + assert ok.startswith("Edited") + assert "return a + b" in (project.build / "app.py").read_text() + assert project.rc.unit_tests_running_context.changed_files == {"app.py"} + + +def test_write_and_delete_are_confined_to_build_folder(project): + denied = agent_tools.write_file({"file_path": str(project.root / "evil.py"), "content": "x"}, project.rc) + assert denied.startswith("Error: write access denied") + assert not (project.root / "evil.py").exists() + + assert agent_tools.write_file({"file_path": "pkg/new.py", "content": "print(1)\n"}, project.rc).startswith("Wrote") + assert (project.build / "pkg" / "new.py").read_text() == "print(1)\n" + assert agent_tools.delete_file({"file_path": "pkg/new.py"}, project.rc).startswith("Deleted") + assert not (project.build / "pkg" / "new.py").exists() + assert project.rc.unit_tests_running_context.changed_files == {os.path.join("pkg", "new.py")} + + +def test_grep_and_ls(project): + hits = agent_tools.grep({"pattern": "def sub"}, project.rc) + assert hits == "app.py:5:def sub(a, b):" + assert agent_tools.grep({"pattern": "nope"}, project.rc).startswith("No matches") + assert agent_tools.grep({"pattern": ""}, project.rc).startswith("Error") + + assert agent_tools.ls_files({}, project.rc).splitlines()[1:] == ["app.py"] + assert agent_tools.ls_files({"pattern": "**/*.py"}, project.rc).endswith("app.py") + + +def test_execute_calls_answers_every_call_and_captures_errors(project, monkeypatch): + def boom(_args, _rc): + raise RuntimeError("kaput") + + monkeypatch.setitem(agent_tools.TOOLS, "boom", boom) + results = agent_tools.execute_calls( + [ + {"id": "1", "name": "read_file", "args": {"file_path": "app.py", "limit": 1}}, + {"id": "2", "name": "unknown_tool", "args": {}}, + {"id": "3", "name": "boom"}, + ], + project.rc, + ) + assert [r["call_id"] for r in results] == ["1", "2", "3"] + assert results[0]["output"].startswith("1: def add") + assert results[1]["output"] == "Error: unknown tool 'unknown_tool'." + assert results[2]["output"] == "Error: tool 'boom' failed: RuntimeError: kaput" + + +def test_run_unit_tests_reports_pass_and_failure(project, monkeypatch): + log = project.root.parent / "log.txt" + log.write_text("FAILED test_x\nCaused by: boom\n") + outcomes = iter([(0, "", "/tmp/pass.txt"), (1, "FAILED test_x", str(log))]) + monkeypatch.setattr(agent_tools.render_utils, "execute_script", lambda *a, **k: next(outcomes)) + project.rc.unittests_script = "run_tests.sh" + context = project.rc.unit_tests_running_context + + assert agent_tools.run_unit_tests({}, project.rc) == {"output": "All unit tests passed."} + assert context.verified_passing and context.verified_passing_log_path == "/tmp/pass.txt" + + failure = agent_tools.run_unit_tests({}, project.rc) + assert failure["output"].startswith(f"Unit tests failed (exit code 1). Full log: {log}") + # raw output goes to the server for condensing, not truncated here + assert failure["test_output"] == "FAILED test_x" + # the full log is outside the build folder but greppable + assert agent_tools.grep({"pattern": "Caused by", "file_path": str(log)}, project.rc).endswith("Caused by: boom") + + +def test_file_change_invalidates_verified_pass_and_read_cache(project): + context = project.rc.unit_tests_running_context + context.verified_passing = True + first = agent_tools.execute_calls([{"id": "1", "name": "read_file", "args": {"file_path": "app.py"}}], project.rc) + repeat = agent_tools.execute_calls([{"id": "2", "name": "read_file", "args": {"file_path": "app.py"}}], project.rc) + assert first[0]["output"].startswith("1: def add") + assert repeat[0]["output"].startswith("Same call as an earlier one") + + agent_tools.write_file({"file_path": "other.py", "content": "y = 1\n"}, project.rc) + assert context.verified_passing is False + again = agent_tools.execute_calls([{"id": "3", "name": "read_file", "args": {"file_path": "app.py"}}], project.rc) + assert again[0]["output"].startswith("1: def add") + + +def test_edit_file_returns_the_edited_region(project): + out = agent_tools.edit_file( + { + "file_path": "app.py", + "search": "def sub(a, b):\n return a - b", + "replace": "def sub(a, b):\n return b", + }, + project.rc, + ) + assert out.startswith("Edited") and "5: def sub(a, b):\n6: return b" in out + assert "2: return a - b" in out and "1: def add" not in out + + +def test_grep_context_lines_and_include(project): + (project.build / "notes.txt").write_text("def sub is documented here\n") + hits = agent_tools.grep({"pattern": "def sub", "context_lines": 1, "include": "*.py"}, project.rc) + assert "notes.txt" not in hits + assert "app.py-4-" in hits and "app.py:5:def sub(a, b):" in hits and "app.py-6-" in hits + + +def test_bound_truncates_long_lines_and_large_output(): + bounded = agent_tools._bound("a" * (agent_tools.MAX_LINE_CHARS + 5) + "\n" + "b\n" * 40_000) + assert "[line truncated]" in bounded and "[truncated" in bounded + assert len(bounded) < agent_tools.MAX_OUTPUT_CHARS + 200 diff --git a/tests/test_fix_unit_tests_action.py b/tests/test_fix_unit_tests_action.py new file mode 100644 index 00000000..27b9bea5 --- /dev/null +++ b/tests/test_fix_unit_tests_action.py @@ -0,0 +1,239 @@ +"""Tests for the agentic FixUnitTests action driving a scripted fake API.""" + +from types import SimpleNamespace + +import pytest +import requests + +import plain_spec +from render_machine.actions.fix_unit_tests import MAX_AGENT_TURNS_PER_ATTEMPT, MAX_RELEVANT_FILES_CHARS, FixUnitTests +from render_machine.actions.run_unit_tests import RunUnitTests +from render_machine.render_context import RenderContext +from render_machine.render_types import ScriptExecutionHistory, UnitTestsRunningContext + + +class FakeRenderContext(SimpleNamespace): + """A render context with just the attributes FixUnitTests uses, and the real session lookup.""" + + unit_tests_agent_session = RenderContext.unit_tests_agent_session + + +class FakeAPI: + def __init__(self, responses): + self.responses = list(responses) + self.calls = [] + + def agent_start(self, task_type, task_params, frid, module_name, run_state): + self.calls.append(("start", task_type, task_params, frid, module_name)) + return self.responses.pop(0) + + def agent_continue(self, session_id, tool_results, frid, module_name, run_state): + self.calls.append(("continue", session_id, tool_results, frid, module_name)) + response = self.responses.pop(0) + if isinstance(response, Exception): + raise response + return response + + +def _tool_calls(*calls): + return {"session_id": "s1", "status": "tool_calls", "calls": list(calls)} + + +@pytest.fixture +def render_context(tmp_path, monkeypatch): + build = tmp_path / "build" + build.mkdir() + (build / "a.py").write_text("x = 1\n") + monkeypatch.chdir(tmp_path) + plain_source_tree = {"spec": True} + specifications = { + plain_spec.DEFINITIONS: ["- :Foo: is a thing."], + plain_spec.NON_FUNCTIONAL_REQUIREMENTS: ["- Python 3.11."], + plain_spec.FUNCTIONAL_REQUIREMENTS: ["- Old feature.", "- New feature."], + } + monkeypatch.setattr(plain_spec, "get_specifications_for_frid", lambda tree, frid: (specifications, None)) + return FakeRenderContext( + codeplain_api=None, + build_folder=str(build), + module_name="m", + run_state=object(), + plain_source_tree=plain_source_tree, + unittests_script=None, + test_script_timeout=None, + stop_event=None, + frid_context=SimpleNamespace(frid="2", linked_resources={"schema.json": "{}"}, changed_files={"a.py"}), + unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), + conformance_tests_running_context=None, + script_execution_history=ScriptExecutionHistory(), + get_required_modules_functionalities=lambda: {"base": ["- Base feature."]}, + ) + + +def test_first_attempt_starts_session_runs_tools_and_stops_at_submit_fix(render_context): + api = FakeAPI( + [ + _tool_calls({"id": "c1", "name": "read_file", "args": {"file_path": "a.py"}}), + _tool_calls( + {"id": "c2", "name": "edit_file", "args": {"file_path": "a.py", "search": "x = 1", "replace": "x = 2"}}, + {"id": "c3", "name": "submit_fix", "args": {"root_cause": "off by one", "changes_made": "x = 2"}}, + ), + ] + ) + render_context.codeplain_api = api + + outcome, payload = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED test_a"}) + + assert (outcome, payload) == (FixUnitTests.SUCCESSFUL_OUTCOME, None) + kind, task_type, task_params, frid, module_name = api.calls[0] + assert (kind, task_type, frid, module_name) == ("start", "fix_unit_tests", "2", "m") + assert task_params["unittests_issue"] == "FAILED test_a" + assert task_params["definitions"] == "- :Foo: is a thing." + assert task_params["linked_resources"] == {"schema.json": "{}"} + assert ( + "### Module: base (Already Implemented, for context)\n- Base feature." in task_params["functional_requirements"] + ) + assert "### Module: m (Already Implemented, for context)\n- Old feature." in task_params["functional_requirements"] + assert task_params["functional_requirements"].endswith( + "### Module: m (Currently Being Implemented)\n- New feature." + ) + + # the read_file result went back to the server; the edit was applied locally + assert api.calls[1][0] == "continue" and api.calls[1][2][0]["call_id"] == "c1" + assert "1: x = 1" in api.calls[1][2][0]["output"] + assert open(render_context.build_folder + "/a.py").read() == "x = 2\n" + + context = render_context.unit_tests_running_context + session = context.agent_session + assert session.session_id == "s1" + assert session.pending_submit_call_id == "c3" + assert [r["call_id"] for r in session.pending_tool_results] == ["c2"] + assert context.changed_files == {"a.py"} + assert "conformance_tests_fixes" not in task_params + + +def test_second_attempt_continues_session_answering_submit_fix(render_context): + context = render_context.unit_tests_running_context + context.agent_used_in_this_loop = True + session = context.agent_session + session.session_id = "s1" + session.pending_submit_call_id = "c3" + session.pending_tool_results = [{"call_id": "c2", "output": "Edited"}] + api = FakeAPI([_tool_calls({"id": "c4", "name": "submit_fix", "args": {"changes_made": "again"}})]) + render_context.codeplain_api = api + + outcome, _ = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED test_b"}) + + assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME + kind, session_id, tool_results, frid, module_name = api.calls[0] + assert (kind, session_id, frid, module_name) == ("continue", "s1", "2", "m") + assert tool_results[0] == {"call_id": "c2", "output": "Edited"} + assert tool_results[1]["call_id"] == "c3" and "still fail" in tool_results[1]["output"] + assert tool_results[1]["test_output"] == "FAILED test_b" + assert "conformance_tests_fixes" not in tool_results[1] + assert session.session_id == "s1" and session.pending_submit_call_id == "c4" + assert session.pending_tool_results == [] + + +@pytest.mark.parametrize( + "final_response", + [ + {"session_id": "s1", "status": "completed", "result": "done"}, + {"session_id": "s1", "status": "failed", "error": "x"}, + ], +) +def test_session_ending_without_submission_resets_the_session(render_context, final_response): + render_context.codeplain_api = FakeAPI([final_response]) + outcome, _ = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME + session = render_context.unit_tests_running_context.agent_session + assert session.session_id is None and session.pending_submit_call_id is None + + +def test_turn_cap_per_attempt_resets_the_session(render_context): + call = {"id": "c", "name": "ls_files", "args": {}} + render_context.codeplain_api = FakeAPI([_tool_calls(call)] * (MAX_AGENT_TURNS_PER_ATTEMPT + 1)) + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + assert len(render_context.codeplain_api.calls) == MAX_AGENT_TURNS_PER_ATTEMPT + 1 + assert render_context.unit_tests_running_context.agent_session.session_id is None + + +def test_missing_issue_is_an_internal_error(render_context): + from plain2code_exceptions import InternalClientError + + with pytest.raises(InternalClientError): + FixUnitTests().execute(render_context, {}) + + +def test_first_turn_is_seeded_with_file_tree_relevant_files_and_log_path(render_context, tmp_path): + (tmp_path / "build" / "tests").mkdir() + (tmp_path / "build" / "tests" / "test_a.py").write_text("assert True\n") + log = tmp_path / "unit.log" + log.write_text("full log\nCaused by: boom\n") + render_context.script_execution_history.latest_unit_test_output_path = str(log) + api = FakeAPI([{"session_id": "s1", "status": "completed", "result": "done"}]) + render_context.codeplain_api = api + + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + + task_params = api.calls[0][2] + assert task_params["file_tree"].split("\n") == ["a.py", "tests/test_a.py"] + assert task_params["relevant_files"] == {"a.py": "x = 1\n"} + assert task_params["unittests_log_path"] == str(log) + assert "previous_session_id" not in task_params + # the agent may grep the full log although it is outside the build folder and project root + assert str(log) in render_context.unit_tests_running_context.agent_session.readable_log_paths + + +def test_relevant_files_stay_within_budget(tmp_path): + (tmp_path / "small.py").write_text("s") + (tmp_path / "big.py").write_text("b" * MAX_RELEVANT_FILES_CHARS) + assert FixUnitTests._relevant_files(str(tmp_path), {"small.py", "big.py", "deleted.py"}) == {"small.py": "s"} + + +def test_new_session_after_abandoned_one_references_it(render_context): + call = {"id": "c", "name": "ls_files", "args": {}} + render_context.codeplain_api = FakeAPI([_tool_calls(call)] * (MAX_AGENT_TURNS_PER_ATTEMPT + 1)) + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + assert render_context.unit_tests_running_context.agent_session.previous_session_id == "s1" + + api = FakeAPI([{"session_id": "s2", "status": "completed", "result": "done"}]) + render_context.codeplain_api = api + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED again"}) + assert api.calls[0][0] == "start" and api.calls[0][2]["previous_session_id"] == "s1" + + +def test_run_unit_tests_action_skips_the_suite_after_a_verified_agent_run(render_context, monkeypatch): + import render_machine.render_utils as render_utils + + context = render_context.unit_tests_running_context + context.verified_passing, context.verified_passing_log_path = True, "/logs/pass.log" + monkeypatch.setattr(render_utils, "execute_script", lambda *a, **k: pytest.fail("suite must not run")) + + assert RunUnitTests().execute(render_context, None) == (RunUnitTests.SUCCESSFUL_OUTCOME, None) + assert context.verified_passing is False + assert render_context.script_execution_history.latest_unit_test_output_path == "/logs/pass.log" + + +def test_expired_session_is_replaced_by_a_new_one(render_context): + context = render_context.unit_tests_running_context + context.agent_used_in_this_loop = True + session = context.agent_session + session.session_id, session.pending_submit_call_id = "expired", "c3" + not_found = requests.exceptions.HTTPError(response=SimpleNamespace(status_code=404)) + api = FakeAPI([not_found, {"session_id": "s2", "status": "completed", "result": "done"}]) + render_context.codeplain_api = api + + outcome, _ = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED test_b"}) + + assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME + assert [call[0] for call in api.calls] == ["continue", "start"] + assert api.calls[1][2]["unittests_issue"] == "FAILED test_b" + + +def test_other_http_errors_on_continue_are_not_swallowed(render_context): + context = render_context.unit_tests_running_context + context.agent_used_in_this_loop = True + context.agent_session.session_id, context.agent_session.pending_submit_call_id = "s1", "c3" + render_context.codeplain_api = FakeAPI([requests.exceptions.HTTPError(response=SimpleNamespace(status_code=500))]) + with pytest.raises(requests.exceptions.HTTPError): + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) diff --git a/tests/test_fix_unit_tests_conformance_context.py b/tests/test_fix_unit_tests_conformance_context.py index a2fe3a1e..7d29f07d 100644 --- a/tests/test_fix_unit_tests_conformance_context.py +++ b/tests/test_fix_unit_tests_conformance_context.py @@ -12,25 +12,44 @@ import plain_spec from memory_management import MemoryManager -from render_machine.actions import fix_unit_tests as fix_unit_tests_module from render_machine.actions.fix_conformance_test import FixConformanceTest from render_machine.actions.fix_unit_tests import FixUnitTests from render_machine.implementation_code_helpers import ImplementationCodeHelpers -from render_machine.render_types import ConformanceTestsRunningContext, UnitTestsRunningContext +from render_machine.render_context import RenderContext +from render_machine.render_types import ConformanceTestsRunningContext, ScriptExecutionHistory, UnitTestsRunningContext class FakeCodeplainAPI: - def __init__(self, conformance_fix_response=None, unittests_fix_response=None): + """Conformance fixes return a scripted response; every agent session submits a fix on its first turn.""" + + def __init__(self, conformance_fix_response=None): self.conformance_fix_response = conformance_fix_response - self.unittests_fix_response = unittests_fix_response or {} - self.unittests_fix_calls = [] + self.agent_calls = [] + self.sessions_started = 0 def fix_conformance_tests_issue(self, *args, **kwargs): return self.conformance_fix_response - def fix_unittests_issue(self, *args, **kwargs): - self.unittests_fix_calls.append(kwargs) - return self.unittests_fix_response + def agent_start(self, task_type, task_params, frid, module_name, run_state): + self.sessions_started += 1 + self.agent_calls.append(("start", task_params)) + return self._submit(f"s{self.sessions_started}") + + def agent_continue(self, session_id, tool_results, frid, module_name, run_state): + self.agent_calls.append(("continue", session_id, tool_results)) + return self._submit(session_id) + + def _submit(self, session_id): + call_id = f"submit-{len(self.agent_calls)}" + return { + "session_id": session_id, + "status": "tool_calls", + "calls": [{"id": call_id, "name": "submit_fix", "args": {"changes_made": "fixed"}}], + } + + +class FakeRenderContext(SimpleNamespace): + unit_tests_agent_session = RenderContext.unit_tests_agent_session class FakeConformanceTests: @@ -59,7 +78,11 @@ def memory_folder(): def isolate_from_git_and_console(monkeypatch): monkeypatch.setattr(ImplementationCodeHelpers, "get_code_diff", staticmethod(lambda *args: {})) monkeypatch.setattr(plain_spec, "collect_linked_resources", lambda *args: None) - monkeypatch.setattr(fix_unit_tests_module.render_utils, "print_inputs", lambda *args: None) + monkeypatch.setattr( + plain_spec, + "get_specifications_for_frid", + lambda tree, frid: ({plain_spec.FUNCTIONAL_REQUIREMENTS: ["- Add numbers."]}, None), + ) def make_conformance_context(): @@ -76,7 +99,7 @@ def make_conformance_context(): def make_render_context(api, build_folder, memory_folder, conformance_tests_running_context): - return SimpleNamespace( + return FakeRenderContext( codeplain_api=api, build_folder=build_folder, memory_manager=MemoryManager(api, memory_folder), @@ -86,9 +109,11 @@ def make_render_context(api, build_folder, memory_folder, conformance_tests_runn plain_source_tree={}, module_name="mod", required_modules=None, - frid_context=SimpleNamespace(frid="1", linked_resources={}), + frid_context=SimpleNamespace(frid="1", linked_resources={}, changed_files=set()), get_required_modules_functionalities=lambda: {}, run_state=SimpleNamespace(render_id="test-render-id", unittest_batch_id=1), + unittests_script=None, + script_execution_history=ScriptExecutionHistory(), ) @@ -191,34 +216,104 @@ def test_implementation_fix_with_no_files_is_not_remembered(build_folder, memory assert ctx.implementation_code_fixes == [] -def run_unit_tests_fix(render_context): - return FixUnitTests().execute(render_context, {"previous_unittests_issue": "1 failed"}) +def run_unit_tests_fix(render_context, issue="1 failed"): + return FixUnitTests().execute(render_context, {"previous_unittests_issue": issue}) + + +def start_new_unit_test_loop(render_context): + """What RenderContext.start_unittests_processing does when the unit tests are run again.""" + render_context.unit_tests_running_context = UnitTestsRunningContext(fix_attempts=0) + + +def conformance_fix(number): + return { + "hypothesis": f"hypothesis {number}", + "approach": f"approach {number}", + "code_diff": {"app.py": f"+{number}"}, + } -def test_unit_tests_fix_forwards_conformance_fixes(build_folder, memory_folder): - api = FakeCodeplainAPI(unittests_fix_response={"test_app.py": "def test_add(): pass\n"}) +def test_first_unit_test_loop_of_conformance_phase_seeds_the_session_with_the_fixes(build_folder, memory_folder): + api = FakeCodeplainAPI() ctx = make_conformance_context() - ctx.implementation_code_fixes.append( - {"hypothesis": "off by one", "approach": "add one", "code_diff": {"app.py": "+ return a + b + 1"}} - ) + ctx.implementation_code_fixes.append(conformance_fix(1)) render_context = make_render_context(api, build_folder, memory_folder, ctx) outcome, _ = run_unit_tests_fix(render_context) assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME - assert len(api.unittests_fix_calls) == 1 - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] == ctx.implementation_code_fixes + kind, task_params = api.agent_calls[0] + assert kind == "start" + assert task_params["conformance_tests_fixes"] == ctx.implementation_code_fixes # The forwarded list is a copy, so later conformance fixes do not mutate what was sent. - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is not ctx.implementation_code_fixes + assert task_params["conformance_tests_fixes"] is not ctx.implementation_code_fixes + # The file the conformance fix changed is seeded although the FRID did not change it. + assert "app.py" in task_params["relevant_files"] + assert ctx.unit_tests_agent_session.session_id == "s1" -def test_unit_tests_fix_outside_conformance_phase_sends_no_fixes(build_folder, memory_folder): +def test_next_unit_test_loop_of_conformance_phase_continues_the_session_with_only_new_fixes( + build_folder, memory_folder +): api = FakeCodeplainAPI() - render_context = make_render_context(api, build_folder, memory_folder, None) + ctx = make_conformance_context() + ctx.implementation_code_fixes.append(conformance_fix(1)) + render_context = make_render_context(api, build_folder, memory_folder, ctx) + run_unit_tests_fix(render_context) + + # The fix was accepted; the conformance tests fixer changes the code again and the unit tests fail again. + ctx.implementation_code_fixes.append(conformance_fix(2)) + start_new_unit_test_loop(render_context) + run_unit_tests_fix(render_context, issue="2 failed") + + assert api.sessions_started == 1 + kind, session_id, tool_results = api.agent_calls[1] + assert (kind, session_id) == ("continue", "s1") + submit_answer = tool_results[-1] + assert submit_answer["call_id"] == "submit-1" + assert submit_answer["output"].startswith("Your fix was accepted: the unit tests passed.") + assert "Conformance Tests Fix below" in submit_answer["output"] + assert submit_answer["conformance_tests_fixes"] == [conformance_fix(2)] + assert submit_answer["test_output"] == "2 failed" + +def test_retry_within_a_unit_test_loop_says_the_fix_did_not_work(build_folder, memory_folder): + api = FakeCodeplainAPI() + ctx = make_conformance_context() + ctx.implementation_code_fixes.append(conformance_fix(1)) + render_context = make_render_context(api, build_folder, memory_folder, ctx) + run_unit_tests_fix(render_context) + + run_unit_tests_fix(render_context, issue="still failing") + + submit_answer = api.agent_calls[1][2][-1] + assert "still fail" in submit_answer["output"] + # Fix 1 was already shown to the session, so it is not sent again. + assert "conformance_tests_fixes" not in submit_answer + + +def test_new_conformance_phase_starts_a_new_session(build_folder, memory_folder): + api = FakeCodeplainAPI() + render_context = make_render_context(api, build_folder, memory_folder, make_conformance_context()) + run_unit_tests_fix(render_context) + + # E.g. the functionality is re-rendered from scratch: the conformance tests running context is recreated. + render_context.conformance_tests_running_context = make_conformance_context() + start_new_unit_test_loop(render_context) + run_unit_tests_fix(render_context) + + assert [call[0] for call in api.agent_calls] == ["start", "start"] + + +def test_unit_tests_outside_conformance_phase_get_no_fixes_and_a_session_per_loop(build_folder, memory_folder): + api = FakeCodeplainAPI() + render_context = make_render_context(api, build_folder, memory_folder, None) + run_unit_tests_fix(render_context) + start_new_unit_test_loop(render_context) run_unit_tests_fix(render_context) - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is None + assert [call[0] for call in api.agent_calls] == ["start", "start"] + assert all("conformance_tests_fixes" not in call[1] for call in api.agent_calls) def test_unit_tests_fix_in_conformance_phase_without_implementation_changes_sends_no_fixes(build_folder, memory_folder): @@ -227,4 +322,4 @@ def test_unit_tests_fix_in_conformance_phase_without_implementation_changes_send run_unit_tests_fix(render_context) - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is None + assert "conformance_tests_fixes" not in api.agent_calls[0][1]