From 4c2264a3f828e1e7b4653aea79e81260808e66ef Mon Sep 17 00:00:00 2001 From: Kavya Sree Kaitepalli Date: Thu, 9 Jul 2026 10:08:50 +0000 Subject: [PATCH 1/3] feat: integrate LogAnalysisBot for enhanced failure analysis in auto_memory --- src/microbots/auto_memory/analyzer.py | 166 +++++++++++++++------- src/microbots/auto_memory/config.py | 21 +++ src/microbots/auto_memory/orchestrator.py | 3 + 3 files changed, 139 insertions(+), 51 deletions(-) diff --git a/src/microbots/auto_memory/analyzer.py b/src/microbots/auto_memory/analyzer.py index 2ca630c..e6ab941 100644 --- a/src/microbots/auto_memory/analyzer.py +++ b/src/microbots/auto_memory/analyzer.py @@ -1,26 +1,37 @@ -"""Failure analysis utilities that convert callback results into structured feedback.""" +"""Failure analysis that uses LogAnalysisBot to turn callback failures into feedback.""" from __future__ import annotations +import tempfile +from logging import getLogger from pathlib import Path from microbots.auto_memory.callbacks import CallbackResult from microbots.auto_memory.data_models import Feedback +from microbots.bot.LogAnalysisBot import LogAnalysisBot -# Maximum bytes read from a log tail when building root-cause snippets. -_MAX_LOG_BYTES = 4096 +logger = getLogger(__name__) def analyze_failure( callback_results: list[CallbackResult], candidate_path: Path, iteration_idx: int, + *, + analyzer_model: str, + analyzer_max_iterations: int, + analyzer_timeout_s: int, ) -> Feedback: - """Produce a :class:`~microbots.auto_memory.data_models.Feedback` from callback results. + """Produce a :class:`~microbots.auto_memory.data_models.Feedback` via LogAnalysisBot. - Inspects every :class:`~microbots.auto_memory.callbacks.CallbackResult` - and assembles a structured failure report that the agent can read on - the next iteration. + Combines the stdout/stderr of every failed callback into a single log + file and hands it to a fresh :class:`~microbots.bot.LogAnalysisBot.LogAnalysisBot` + with the candidate directory mounted read-only. The bot's final answer + becomes the sole entry in :attr:`Feedback.root_causes`. + + On no failures, returns a minimal ``All callbacks passed.`` feedback + without invoking the bot. On bot failure, returns a feedback whose + ``root_causes`` describes the analyzer error. Parameters ---------- @@ -28,77 +39,130 @@ def analyze_failure( Results returned by :meth:`~microbots.auto_memory.callbacks.ShellCallbackRunner.run_all`. candidate_path : Path - Path to the candidate output for this iteration. Reserved for - future use (e.g. static analysis); not read directly at present. + Path to the candidate output for this iteration; mounted read-only + into the bot so it can correlate log entries with source. iteration_idx : int Zero-based index of the iteration that produced these results. + analyzer_model : str + LiteLLM model identifier for :class:`~microbots.bot.LogAnalysisBot.LogAnalysisBot`. + analyzer_max_iterations : int + Maximum bot iterations to run. + analyzer_timeout_s : int + Bot timeout in seconds. Returns ------- Feedback Structured summary with ``validator_failures``, ``root_causes``, - ``suggested_actions``, and a human-readable ``summary`` string. + and a human-readable ``summary`` string. """ failed = [r for r in callback_results if not r.passed] - validator_failures: list[str] = [r.spec.name for r in failed] - root_causes: list[str] = [] - suggested_actions: list[str] = [] - for result in failed: - if result.timed_out: - root_causes.append( - f"{result.spec.name}: timed out after {result.spec.timeout_s}s" - ) - suggested_actions.append( - f"Reduce runtime of {result.spec.name!r} or increase its timeout." - ) - else: - log_snippet = _read_tail(result.stderr_path) or _read_tail(result.stdout_path) - if log_snippet: - root_causes.append(f"{result.spec.name}: {log_snippet[:200]}") - else: - root_causes.append( - f"{result.spec.name}: exited with code {result.return_code}" - f" (expected {result.spec.expected_return_code})" - ) - - if failed: - names = ", ".join(validator_failures) - summary = f"{len(failed)} of {len(callback_results)} callback(s) failed: {names}" - else: - summary = "All callbacks passed." + if not failed: + return Feedback(iteration_idx=iteration_idx, summary="All callbacks passed.") + + summary = ( + f"{len(failed)} of {len(callback_results)} callback(s) failed: " + f"{', '.join(validator_failures)}" + ) + + # LogAnalysisBot needs a directory for its read-only mount. + mount_folder = ( + candidate_path + if candidate_path.exists() and candidate_path.is_dir() + else candidate_path.parent + ) + + combined_log = _write_combined_log(failed) + try: + bot = LogAnalysisBot( + model=analyzer_model, + folder_to_mount=str(mount_folder), + ) + bot_result = bot.run( + file_name=str(combined_log), + max_iterations=analyzer_max_iterations, + timeout_in_seconds=analyzer_timeout_s, + ) + except Exception as exc: + logger.warning("LogAnalysisBot invocation failed: %s", exc) + return Feedback( + iteration_idx=iteration_idx, + summary=summary, + validator_failures=validator_failures, + root_causes=[f"LLM analyzer error: {exc}"], + ) + finally: + combined_log.unlink(missing_ok=True) + + if bot_result.status and bot_result.result: + # The LLM produces one narrative diagnosis, not a list of causes; + # store it as the summary and leave root_causes empty. + return Feedback( + iteration_idx=iteration_idx, + summary=bot_result.result.strip(), + validator_failures=validator_failures, + ) return Feedback( iteration_idx=iteration_idx, summary=summary, - root_causes=root_causes, validator_failures=validator_failures, - suggested_actions=suggested_actions, + root_causes=[ + f"LLM analyzer did not produce a result: {bot_result.error or 'unknown'}" + ], ) -def _read_tail(path: Path, max_bytes: int = _MAX_LOG_BYTES) -> str: - """Return the last *max_bytes* of *path* as a stripped string, or ``''`` if missing. +def _write_combined_log(failed: list[CallbackResult]) -> Path: + """Write a temp log file combining every failed callback's stdout/stderr. + + Parameters + ---------- + failed : list[CallbackResult] + Results of the callbacks that did not pass. + + Returns + ------- + Path + Path to the newly created combined log file. Caller is responsible + for deleting it. + """ + with tempfile.NamedTemporaryFile( + mode="w", suffix=".log", delete=False, encoding="utf-8" + ) as fh: + for r in failed: + fh.write(f"\n===== callback: {r.spec.name} =====\n") + fh.write( + f"return_code={r.return_code} " + f"expected={r.spec.expected_return_code} " + f"timed_out={r.timed_out} " + f"duration_s={r.duration_s:.2f}\n" + ) + fh.write("--- stderr ---\n") + fh.write(_safe_read(r.stderr_path)) + fh.write("\n--- stdout ---\n") + fh.write(_safe_read(r.stdout_path)) + fh.write("\n") + return Path(fh.name) + + +def _safe_read(path: Path) -> str: + """Return the text content of *path*, or a placeholder on I/O error. Parameters ---------- path : Path - File to read from. - max_bytes : int, optional - Maximum number of bytes to read from the end of the file. + Filesystem path to read. Returns ------- str - Decoded and stripped tail of the file, or an empty string if the - file cannot be opened. + The file's contents decoded as UTF-8 (with replacement for invalid + bytes), or ``""`` if the file cannot be read. """ try: - with path.open("rb") as fh: - fh.seek(0, 2) - size = fh.tell() - fh.seek(max(0, size - max_bytes)) - return fh.read().decode("utf-8", errors="replace").strip() + return path.read_text(encoding="utf-8", errors="replace") except OSError: - return "" + return "" diff --git a/src/microbots/auto_memory/config.py b/src/microbots/auto_memory/config.py index ed1cce3..d4ceb2f 100644 --- a/src/microbots/auto_memory/config.py +++ b/src/microbots/auto_memory/config.py @@ -30,6 +30,11 @@ class TaskConfig: timeout_min: int = 60 per_iteration_timeout: int = 600 # seconds + # --- analyzer (LogAnalysisBot) settings --- + analyzer_model: str = "azure/gpt-4o" + analyzer_max_iterations: int = 20 + analyzer_timeout_s: int = 300 + # ----------------------------------------------------------------------- @classmethod @@ -103,6 +108,9 @@ def load_from_yaml(cls, path: str) -> "TaskConfig": max_iterations=int(data.get("max_iterations", 5)), timeout_min=int(data.get("timeout_min", 60)), per_iteration_timeout=int(data.get("per_iteration_timeout", 600)), + analyzer_model=str(data.get("analyzer_model", "azure/gpt-4o")), + analyzer_max_iterations=int(data.get("analyzer_max_iterations", 20)), + analyzer_timeout_s=int(data.get("analyzer_timeout_s", 300)), ) config.validate() return config @@ -130,6 +138,19 @@ def validate(self) -> None: f"'per_iteration_timeout' must be >= 1, got {self.per_iteration_timeout}" ) + if not self.analyzer_model: + raise ConfigError("'analyzer_model' must not be empty") + + if self.analyzer_max_iterations < 1: + raise ConfigError( + f"'analyzer_max_iterations' must be >= 1, got {self.analyzer_max_iterations}" + ) + + if self.analyzer_timeout_s < 1: + raise ConfigError( + f"'analyzer_timeout_s' must be >= 1, got {self.analyzer_timeout_s}" + ) + if self.output_format not in ("file", "dir", "stdout"): raise ConfigError( f"'output_format' must be 'file', 'dir', or 'stdout', got '{self.output_format}'" diff --git a/src/microbots/auto_memory/orchestrator.py b/src/microbots/auto_memory/orchestrator.py index ed47c00..2f27e2f 100644 --- a/src/microbots/auto_memory/orchestrator.py +++ b/src/microbots/auto_memory/orchestrator.py @@ -380,4 +380,7 @@ def analyze_failure( callback_results=callback_results, candidate_path=candidate_path, iteration_idx=iteration_idx, + analyzer_model=self._config.analyzer_model, + analyzer_max_iterations=self._config.analyzer_max_iterations, + analyzer_timeout_s=self._config.analyzer_timeout_s, ) From a0bd62c7b934ef144f66422c675a6d4a30768389 Mon Sep 17 00:00:00 2001 From: Kavya Sree Kaitepalli Date: Thu, 9 Jul 2026 11:00:19 +0000 Subject: [PATCH 2/3] update model identifiers to use 'azure-openai' and enhance analyzer functionality --- src/microbots/auto_memory/analyzer.py | 62 ++- src/microbots/auto_memory/cli.py | 2 +- src/microbots/auto_memory/config.py | 21 +- src/microbots/auto_memory/orchestrator.py | 2 +- .../auto_memory/runners/writing_bot_runner.py | 4 +- .../runners/test_writing_bot_runner.py | 2 +- test/auto_memory/test_analyzer.py | 441 ++++++++++++++---- test/auto_memory/test_cli.py | 17 +- test/auto_memory/test_config.py | 36 ++ test/auto_memory/test_orchestrator.py | 3 + 10 files changed, 473 insertions(+), 117 deletions(-) diff --git a/src/microbots/auto_memory/analyzer.py b/src/microbots/auto_memory/analyzer.py index e6ab941..57cde1f 100644 --- a/src/microbots/auto_memory/analyzer.py +++ b/src/microbots/auto_memory/analyzer.py @@ -12,6 +12,11 @@ logger = getLogger(__name__) +# Per-stream tail cap when assembling the combined log fed to the LLM +# analyzer. Prevents pathological callback outputs (e.g. verbose pytest +# tracebacks or dumped fixtures) from blowing up model context / cost. +_LOG_TAIL_BYTES = 16 * 1024 + def analyze_failure( callback_results: list[CallbackResult], @@ -25,13 +30,21 @@ def analyze_failure( """Produce a :class:`~microbots.auto_memory.data_models.Feedback` via LogAnalysisBot. Combines the stdout/stderr of every failed callback into a single log - file and hands it to a fresh :class:`~microbots.bot.LogAnalysisBot.LogAnalysisBot` - with the candidate directory mounted read-only. The bot's final answer - becomes the sole entry in :attr:`Feedback.root_causes`. + file and hands it to a fresh + :class:`~microbots.bot.LogAnalysisBot.LogAnalysisBot` with the candidate + directory mounted read-only. The bot's final narrative answer is stored + as :attr:`Feedback.summary`; :attr:`Feedback.root_causes` is left empty + on the success path because the LLM emits a single diagnosis rather + than a discrete list. On no failures, returns a minimal ``All callbacks passed.`` feedback - without invoking the bot. On bot failure, returns a feedback whose - ``root_causes`` describes the analyzer error. + without invoking the bot. If the bot raises or fails to produce a + result, :attr:`Feedback.summary` falls back to a short + ``" of callback(s) failed: ..."`` string and + :attr:`Feedback.root_causes` contains the analyzer error. + + In every non-passing case :attr:`Feedback.validator_failures` lists the + names of the callbacks that did not pass. Parameters ---------- @@ -86,7 +99,7 @@ def analyze_failure( timeout_in_seconds=analyzer_timeout_s, ) except Exception as exc: - logger.warning("LogAnalysisBot invocation failed: %s", exc) + logger.warning("LogAnalysisBot invocation failed: %s", exc, exc_info=True) return Feedback( iteration_idx=iteration_idx, summary=summary, @@ -148,21 +161,48 @@ def _write_combined_log(failed: list[CallbackResult]) -> Path: return Path(fh.name) -def _safe_read(path: Path) -> str: - """Return the text content of *path*, or a placeholder on I/O error. +def _safe_read(path: Path, max_bytes: int = _LOG_TAIL_BYTES) -> str: + """Return the tail of *path*'s text content, or a placeholder on I/O error. + + Only the last ``max_bytes`` bytes are read to bound memory usage and the + downstream LLM analyzer context. When the file exceeds that cap a + ``"\\n"`` marker is prepended so + the model knows the view is partial. Parameters ---------- path : Path Filesystem path to read. + max_bytes : int, optional + Maximum number of trailing bytes to return. Defaults to + ``_LOG_TAIL_BYTES``. A value ``<= 0`` disables the cap. Returns ------- str - The file's contents decoded as UTF-8 (with replacement for invalid - bytes), or ``""`` if the file cannot be read. + The (possibly truncated) file contents decoded as UTF-8 (invalid + bytes replaced), or ``""`` if the file cannot be + read. """ try: - return path.read_text(encoding="utf-8", errors="replace") + if max_bytes <= 0: + data = path.read_bytes() + truncated = False + total = len(data) + else: + size = path.stat().st_size + with path.open("rb") as fh: + if size > max_bytes: + fh.seek(size - max_bytes) + truncated = True + else: + truncated = False + data = fh.read() + total = size except OSError: return "" + + text = data.decode("utf-8", errors="replace") + if truncated: + return f"\n{text}" + return text diff --git a/src/microbots/auto_memory/cli.py b/src/microbots/auto_memory/cli.py index 7f55124..3cccfd1 100644 --- a/src/microbots/auto_memory/cli.py +++ b/src/microbots/auto_memory/cli.py @@ -50,7 +50,7 @@ def run_from_yaml( generated to avoid collisions. model : str Model identifier forwarded to :class:`WritingBotRunner` (required, - keyword-only — e.g. ``"azure/gpt-4o"``). + keyword-only — e.g. ``"azure-openai/gpt-4o"``). Returns ------- diff --git a/src/microbots/auto_memory/config.py b/src/microbots/auto_memory/config.py index d4ceb2f..75e2159 100644 --- a/src/microbots/auto_memory/config.py +++ b/src/microbots/auto_memory/config.py @@ -9,6 +9,7 @@ from microbots.auto_memory.errors import ConfigError from microbots.auto_memory.data_models import CallbackSpec, ReferenceInput +from microbots.constants import ModelProvider logger = getLogger(__name__) @@ -31,7 +32,7 @@ class TaskConfig: per_iteration_timeout: int = 600 # seconds # --- analyzer (LogAnalysisBot) settings --- - analyzer_model: str = "azure/gpt-4o" + analyzer_model: str = "azure-openai/gpt-4o" analyzer_max_iterations: int = 20 analyzer_timeout_s: int = 300 @@ -108,7 +109,7 @@ def load_from_yaml(cls, path: str) -> "TaskConfig": max_iterations=int(data.get("max_iterations", 5)), timeout_min=int(data.get("timeout_min", 60)), per_iteration_timeout=int(data.get("per_iteration_timeout", 600)), - analyzer_model=str(data.get("analyzer_model", "azure/gpt-4o")), + analyzer_model=str(data.get("analyzer_model", "azure-openai/gpt-4o")), analyzer_max_iterations=int(data.get("analyzer_max_iterations", 20)), analyzer_timeout_s=int(data.get("analyzer_timeout_s", 300)), ) @@ -141,6 +142,22 @@ def validate(self) -> None: if not self.analyzer_model: raise ConfigError("'analyzer_model' must not be empty") + # Mirror MicroBot._validate_model_and_provider so we fail fast at + # config load time instead of deferring to a runtime ValueError + # when LogAnalysisBot is instantiated. + if self.analyzer_model.count("/") != 1: + raise ConfigError( + f"'analyzer_model' must be in the format '/', " + f"got '{self.analyzer_model}'" + ) + provider = self.analyzer_model.split("/", 1)[0] + supported = [e.value for e in ModelProvider] + if provider not in supported: + raise ConfigError( + f"'analyzer_model' has unsupported provider '{provider}'; " + f"expected one of {supported}" + ) + if self.analyzer_max_iterations < 1: raise ConfigError( f"'analyzer_max_iterations' must be >= 1, got {self.analyzer_max_iterations}" diff --git a/src/microbots/auto_memory/orchestrator.py b/src/microbots/auto_memory/orchestrator.py index 2f27e2f..752254d 100644 --- a/src/microbots/auto_memory/orchestrator.py +++ b/src/microbots/auto_memory/orchestrator.py @@ -71,7 +71,7 @@ class TrainingLoopOrchestrator: orchestrator = TrainingLoopOrchestrator( config=cfg, - agent_runner=WritingBotRunner(model="azure/gpt-4o"), + agent_runner=WritingBotRunner(model="azure-openai/gpt-4o"), callback_runner=ShellCallbackRunner(), workspace=WorkspaceManager(run_dir=Path("runs/my_task")), ) diff --git a/src/microbots/auto_memory/runners/writing_bot_runner.py b/src/microbots/auto_memory/runners/writing_bot_runner.py index 4f2f231..b413616 100644 --- a/src/microbots/auto_memory/runners/writing_bot_runner.py +++ b/src/microbots/auto_memory/runners/writing_bot_runner.py @@ -24,7 +24,7 @@ class WritingBotRunner: ---------- model : str Model identifier forwarded to ``WritingBot`` (e.g. - ``"azure/gpt-4o"``). + ``"azure-openai/gpt-4o"``). max_iterations : int, optional Maximum number of agent steps per run; forwarded to ``WritingBot.run()``. Defaults to 20. @@ -36,7 +36,7 @@ def __init__(self, model: str, max_iterations: int = 20) -> None: Parameters ---------- model : str - Model identifier forwarded to ``WritingBot`` (e.g. ``"azure/gpt-4o"``). + Model identifier forwarded to ``WritingBot`` (e.g. ``"azure-openai/gpt-4o"``). max_iterations : int, optional Maximum number of agent steps per run. Defaults to 20. """ diff --git a/test/auto_memory/runners/test_writing_bot_runner.py b/test/auto_memory/runners/test_writing_bot_runner.py index 517bc52..d83a583 100644 --- a/test/auto_memory/runners/test_writing_bot_runner.py +++ b/test/auto_memory/runners/test_writing_bot_runner.py @@ -13,7 +13,7 @@ # Helpers # --------------------------------------------------------------------------- -_MODEL = "azure/gpt-4o" +_MODEL = "azure-openai/gpt-4o" _TASK = "Write a summary to /memories/summary.md" diff --git a/test/auto_memory/test_analyzer.py b/test/auto_memory/test_analyzer.py index 00691e7..78882c1 100644 --- a/test/auto_memory/test_analyzer.py +++ b/test/auto_memory/test_analyzer.py @@ -1,7 +1,19 @@ -import pytest +"""Tests for :func:`microbots.auto_memory.analyzer.analyze_failure`. + +The analyzer delegates the failure-diagnosis narrative to +:class:`~microbots.bot.LogAnalysisBot.LogAnalysisBot`; the bot is patched +in these tests to keep them fast and offline. +""" + +from __future__ import annotations + from pathlib import Path +from unittest.mock import MagicMock, patch -from microbots.auto_memory.analyzer import analyze_failure +import pytest + +from microbots.MicroBot import BotRunResult +from microbots.auto_memory.analyzer import _LOG_TAIL_BYTES, _safe_read, analyze_failure from microbots.auto_memory.callbacks import CallbackResult from microbots.auto_memory.data_models import CallbackSpec, Feedback @@ -10,8 +22,17 @@ # Helpers # --------------------------------------------------------------------------- +_ANALYZER_KWARGS = { + "analyzer_model": "azure-openai/gpt-4o", + "analyzer_max_iterations": 5, + "analyzer_timeout_s": 30, +} + + def _spec(name: str = "tests", expected_rc: int = 0, timeout_s: int = 120) -> CallbackSpec: - return CallbackSpec(name=name, command="pytest", timeout_s=timeout_s, expected_return_code=expected_rc) + return CallbackSpec( + name=name, command="pytest", timeout_s=timeout_s, expected_return_code=expected_rc + ) def _result( @@ -40,153 +61,327 @@ def _result( ) +def _patched_bot( + *, + status: bool = True, + result: str | None = "diagnosis narrative", + error: str | None = None, + raises: Exception | None = None, +): + """Return a patcher for LogAnalysisBot with a controllable BotRunResult.""" + bot_instance = MagicMock() + if raises is not None: + bot_instance.run.side_effect = raises + else: + bot_instance.run.return_value = BotRunResult( + status=status, result=result, error=error + ) + return patch( + "microbots.auto_memory.analyzer.LogAnalysisBot", + return_value=bot_instance, + ), bot_instance + + # --------------------------------------------------------------------------- -# All callbacks passed +# All callbacks passed (short-circuit — bot is NOT invoked) # --------------------------------------------------------------------------- + @pytest.mark.unit class TestAnalyzeFailureAllPassed: def test_returns_feedback(self, tmp_path): results = [_result(tmp_path, "a"), _result(tmp_path, "b")] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) + fb = analyze_failure( + results, tmp_path / "cand", iteration_idx=0, **_ANALYZER_KWARGS + ) assert isinstance(fb, Feedback) def test_summary_says_all_passed(self, tmp_path): results = [_result(tmp_path, "a"), _result(tmp_path, "b")] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) + fb = analyze_failure( + results, tmp_path / "cand", iteration_idx=0, **_ANALYZER_KWARGS + ) assert fb.summary == "All callbacks passed." def test_no_validator_failures(self, tmp_path): results = [_result(tmp_path, "tests")] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) + fb = analyze_failure( + results, tmp_path / "cand", iteration_idx=0, **_ANALYZER_KWARGS + ) assert fb.validator_failures == [] def test_no_root_causes(self, tmp_path): results = [_result(tmp_path, "lint")] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) + fb = analyze_failure( + results, tmp_path / "cand", iteration_idx=0, **_ANALYZER_KWARGS + ) assert fb.root_causes == [] def test_iteration_idx_stored(self, tmp_path): results = [_result(tmp_path, "tests")] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=2) + fb = analyze_failure( + results, tmp_path / "cand", iteration_idx=2, **_ANALYZER_KWARGS + ) assert fb.iteration_idx == 2 + def test_bot_not_invoked_when_all_pass(self, tmp_path): + patcher, bot_instance = _patched_bot() + with patcher: + analyze_failure( + [_result(tmp_path, "tests", passed=True)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + bot_instance.run.assert_not_called() + # --------------------------------------------------------------------------- -# Some callbacks failed (non-timeout) +# Some callbacks failed — happy path (bot returns a narrative) # --------------------------------------------------------------------------- + @pytest.mark.unit -class TestAnalyzeFailureWithFailures: +class TestAnalyzeFailureBotSucceeds: + def test_summary_is_bot_result(self, tmp_path): + patcher, _ = _patched_bot(result="Root cause: missing return statement.") + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert fb.summary == "Root cause: missing return statement." + def test_validator_failures_lists_failed_names(self, tmp_path): + patcher, _ = _patched_bot() results = [ _result(tmp_path, "unit_tests", passed=True), _result(tmp_path, "lint", passed=False, return_code=1), ] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert "lint" in fb.validator_failures - assert "unit_tests" not in fb.validator_failures - - def test_summary_includes_count_and_names(self, tmp_path): - results = [ - _result(tmp_path, "a", passed=False, return_code=1), - _result(tmp_path, "b", passed=False, return_code=2), - ] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert "2 of 2" in fb.summary - assert "a" in fb.summary - assert "b" in fb.summary - - def test_root_cause_includes_stderr_snippet(self, tmp_path): - results = [ - _result( - tmp_path, "tests", - passed=False, return_code=1, - stderr_content="AssertionError: expected 42", + with patcher: + fb = analyze_failure( + results, tmp_path / "cand", iteration_idx=0, **_ANALYZER_KWARGS ) - ] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert any("AssertionError" in c for c in fb.root_causes) - - def test_root_cause_falls_back_to_stdout(self, tmp_path): - results = [ - _result( - tmp_path, "tests", - passed=False, return_code=1, - stderr_content="", - stdout_content="FAILED test_something.py::test_add", + assert fb.validator_failures == ["lint"] + + def test_root_causes_empty_on_success(self, tmp_path): + patcher, _ = _patched_bot() + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, ) - ] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert any("FAILED" in c for c in fb.root_causes) - - def test_root_cause_fallback_when_no_logs(self, tmp_path): - result = _result(tmp_path, "mycheck", passed=False, return_code=3, expected_rc=0) - # Wipe the log files so they are empty - result.stderr_path.write_text("") - result.stdout_path.write_text("") - fb = analyze_failure([result], tmp_path / "cand", iteration_idx=0) - assert any("3" in c for c in fb.root_causes) # exit code 3 mentioned - - def test_mixed_pass_fail_summary(self, tmp_path): - results = [ - _result(tmp_path, "unit", passed=True), - _result(tmp_path, "e2e", passed=False, return_code=1), - ] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=1) - assert "1 of 2" in fb.summary + assert fb.root_causes == [] - def test_one_root_cause_per_failed_callback(self, tmp_path): - results = [ - _result(tmp_path, "a", passed=False, return_code=1), - _result(tmp_path, "b", passed=False, return_code=1), - ] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert len(fb.root_causes) == 2 + def test_bot_result_is_stripped(self, tmp_path): + patcher, _ = _patched_bot(result=" narrative with padding \n") + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert fb.summary == "narrative with padding" + + def test_iteration_idx_preserved(self, tmp_path): + patcher, _ = _patched_bot() + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=4, + **_ANALYZER_KWARGS, + ) + assert fb.iteration_idx == 4 + + def test_bot_invoked_with_configured_model_and_limits(self, tmp_path): + patcher, bot_instance = _patched_bot() + with patch( + "microbots.auto_memory.analyzer.LogAnalysisBot", + return_value=bot_instance, + ) as mock_ctor: + analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + analyzer_model="azure-openai/gpt-4o-mini", + analyzer_max_iterations=7, + analyzer_timeout_s=99, + ) + # Constructor received model + mount folder. + ctor_kwargs = mock_ctor.call_args.kwargs + assert ctor_kwargs["model"] == "azure-openai/gpt-4o-mini" + assert "folder_to_mount" in ctor_kwargs + # run() received the configured iteration + timeout. + run_kwargs = bot_instance.run.call_args.kwargs + assert run_kwargs["max_iterations"] == 7 + assert run_kwargs["timeout_in_seconds"] == 99 + + def test_bot_mounts_candidate_dir_when_it_exists(self, tmp_path): + cand_dir = tmp_path / "cand" + cand_dir.mkdir() + patcher, bot_instance = _patched_bot() + with patch( + "microbots.auto_memory.analyzer.LogAnalysisBot", + return_value=bot_instance, + ) as mock_ctor: + analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + cand_dir, + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert mock_ctor.call_args.kwargs["folder_to_mount"] == str(cand_dir) + + def test_bot_mounts_parent_when_candidate_missing(self, tmp_path): + cand_dir = tmp_path / "missing_candidate" # not created + patcher, bot_instance = _patched_bot() + with patch( + "microbots.auto_memory.analyzer.LogAnalysisBot", + return_value=bot_instance, + ) as mock_ctor: + analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + cand_dir, + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert mock_ctor.call_args.kwargs["folder_to_mount"] == str(tmp_path) # --------------------------------------------------------------------------- -# Timed-out callbacks +# Bot failed to produce a result # --------------------------------------------------------------------------- + @pytest.mark.unit -class TestAnalyzeFailureTimeout: - def test_timed_out_callback_in_validator_failures(self, tmp_path): - results = [_result(tmp_path, "slow", passed=False, return_code=-1, timed_out=True, timeout_s=30)] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert "slow" in fb.validator_failures +class TestAnalyzeFailureBotNoResult: + def test_root_cause_reports_unknown_when_no_error(self, tmp_path): + patcher, _ = _patched_bot(status=False, result=None, error=None) + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert any("unknown" in c for c in fb.root_causes) + + def test_root_cause_includes_bot_error(self, tmp_path): + patcher, _ = _patched_bot(status=False, result=None, error="bot bailed") + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert any("bot bailed" in c for c in fb.root_causes) + + def test_summary_falls_back_to_count(self, tmp_path): + patcher, _ = _patched_bot(status=False, result=None, error="bot bailed") + with patcher: + fb = analyze_failure( + [ + _result(tmp_path, "a", passed=False, return_code=1), + _result(tmp_path, "b", passed=False, return_code=2), + ], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert "2 of 2" in fb.summary + assert "a" in fb.summary and "b" in fb.summary + + def test_validator_failures_still_populated(self, tmp_path): + patcher, _ = _patched_bot(status=False, result=None, error="x") + with patcher: + fb = analyze_failure( + [_result(tmp_path, "lint", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert fb.validator_failures == ["lint"] + + def test_empty_result_treated_as_no_result(self, tmp_path): + # status=True but result="" is falsy → same "did not produce a result" path. + patcher, _ = _patched_bot(status=True, result="", error=None) + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert any("did not produce a result" in c for c in fb.root_causes) - def test_timed_out_root_cause_mentions_timeout(self, tmp_path): - results = [_result(tmp_path, "slow", passed=False, return_code=-1, timed_out=True, timeout_s=30)] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert any("timed out" in c for c in fb.root_causes) - def test_timed_out_root_cause_mentions_timeout_seconds(self, tmp_path): - results = [_result(tmp_path, "slow", passed=False, return_code=-1, timed_out=True, timeout_s=45)] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert any("45" in c for c in fb.root_causes) +# --------------------------------------------------------------------------- +# Bot invocation itself raised +# --------------------------------------------------------------------------- - def test_timed_out_suggested_action_present(self, tmp_path): - results = [_result(tmp_path, "slow", passed=False, return_code=-1, timed_out=True)] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert len(fb.suggested_actions) >= 1 - def test_timed_out_suggested_action_mentions_callback_name(self, tmp_path): - results = [_result(tmp_path, "bigtest", passed=False, return_code=-1, timed_out=True)] - fb = analyze_failure(results, tmp_path / "cand", iteration_idx=0) - assert any("bigtest" in a for a in fb.suggested_actions) +@pytest.mark.unit +class TestAnalyzeFailureBotRaises: + def test_exception_is_captured_in_root_causes(self, tmp_path): + patcher, _ = _patched_bot(raises=RuntimeError("network down")) + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert any("network down" in c for c in fb.root_causes) + assert any("LLM analyzer error" in c for c in fb.root_causes) + + def test_summary_falls_back_on_exception(self, tmp_path): + patcher, _ = _patched_bot(raises=RuntimeError("boom")) + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert "1 of 1" in fb.summary + assert "tests" in fb.summary + + def test_validator_failures_still_populated_on_exception(self, tmp_path): + patcher, _ = _patched_bot(raises=RuntimeError("boom")) + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=1, + **_ANALYZER_KWARGS, + ) + assert fb.validator_failures == ["tests"] + assert fb.iteration_idx == 1 # --------------------------------------------------------------------------- -# _read_tail — OSError fallback +# _safe_read — OSError fallback # --------------------------------------------------------------------------- + @pytest.mark.unit -class TestReadTailOSError: - def test_missing_log_files_fall_back_to_exit_code_root_cause(self, tmp_path): - """OSError on missing log files must not propagate; root cause falls back to exit code.""" +class TestSafeReadOSError: + def test_missing_log_files_do_not_raise(self, tmp_path): + """OSError on missing log files must be swallowed by _safe_read. + + We drive it through the public API by pointing a failed + CallbackResult at non-existent log paths and letting the analyzer + combine them into its temp log before handing it to the (mocked) + bot. + """ spec = _spec("mycheck", expected_rc=0) - # Create a result whose log paths point to non-existent files. result = CallbackResult( spec=spec, return_code=5, @@ -194,5 +389,57 @@ def test_missing_log_files_fall_back_to_exit_code_root_cause(self, tmp_path): stderr_path=tmp_path / "nonexistent.stderr", passed=False, ) - fb = analyze_failure([result], tmp_path / "cand", iteration_idx=0) - assert any("5" in c for c in fb.root_causes) + patcher, _ = _patched_bot(result="ok") + with patcher: + fb = analyze_failure( + [result], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert fb.validator_failures == ["mycheck"] + assert fb.summary == "ok" + + +# --------------------------------------------------------------------------- +# _safe_read — tail-truncation cap +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestSafeReadTailCap: + def test_small_file_returned_verbatim(self, tmp_path): + p = tmp_path / "small.log" + p.write_text("hello world") + assert _safe_read(p) == "hello world" + + def test_default_cap_is_applied(self, tmp_path): + p = tmp_path / "big.log" + # Head + tail with a distinct marker so we can verify only the tail returned. + payload = ("A" * (_LOG_TAIL_BYTES + 100)) + "TAIL_MARKER" + p.write_text(payload) + out = _safe_read(p) + assert out.startswith("\n") + assert out.endswith("xxxxxxxEND") + + def test_cap_zero_disables_truncation(self, tmp_path): + p = tmp_path / "full.log" + payload = "y" * (_LOG_TAIL_BYTES + 50) + p.write_text(payload) + assert _safe_read(p, max_bytes=0) == payload + + def test_truncation_marker_reports_total_size(self, tmp_path): + p = tmp_path / "sized.log" + p.write_text("z" * 500) + out = _safe_read(p, max_bytes=100) + assert "of 500>" in out diff --git a/test/auto_memory/test_cli.py b/test/auto_memory/test_cli.py index 04d966d..889ccf1 100644 --- a/test/auto_memory/test_cli.py +++ b/test/auto_memory/test_cli.py @@ -13,7 +13,7 @@ from microbots.auto_memory.orchestrator import RunSummary from microbots.MicroBot import BotRunResult -_MODEL = "azure/gpt-4o" +_MODEL = "azure-openai/gpt-4o" # --------------------------------------------------------------------------- @@ -63,6 +63,18 @@ def _mock_writing_bot(status: bool = True, error: str | None = None): ), bot_instance +def _mock_log_analysis_bot(result: str = "diagnosis narrative"): + """Patch LogAnalysisBot so failure analysis never hits a real LLM endpoint.""" + bot_instance = MagicMock() + bot_instance.run.return_value = BotRunResult( + status=True, result=result, error=None + ) + return patch( + "microbots.auto_memory.analyzer.LogAnalysisBot", + return_value=bot_instance, + ), bot_instance + + # --------------------------------------------------------------------------- # End-to-end # --------------------------------------------------------------------------- @@ -159,8 +171,9 @@ def test_failing_callbacks_persist_feedback_and_reach_limit(self, tmp_path): yaml_path = _write_yaml(tmp_path, _TASK_YAML_FAILING) workdir = tmp_path / "workdir" bot_patch, _ = _mock_writing_bot() + analyzer_patch, _ = _mock_log_analysis_bot() - with bot_patch, patch( + with bot_patch, analyzer_patch, patch( "microbots.auto_memory.runners.writing_bot_runner.MemoryTool" ): summary = run_from_yaml( diff --git a/test/auto_memory/test_config.py b/test/auto_memory/test_config.py index f7a5100..412db1d 100644 --- a/test/auto_memory/test_config.py +++ b/test/auto_memory/test_config.py @@ -165,6 +165,42 @@ def test_per_iteration_timeout_zero(self): with pytest.raises(ConfigError, match="per_iteration_timeout"): cfg.validate() + def test_analyzer_model_empty(self): + cfg = self._base() + cfg.analyzer_model = "" + with pytest.raises(ConfigError, match="analyzer_model"): + cfg.validate() + + def test_analyzer_model_missing_slash(self): + cfg = self._base() + cfg.analyzer_model = "gpt-4o" + with pytest.raises(ConfigError, match="/"): + cfg.validate() + + def test_analyzer_model_too_many_slashes(self): + cfg = self._base() + cfg.analyzer_model = "azure-openai/gpt-4o/extra" + with pytest.raises(ConfigError, match="/"): + cfg.validate() + + def test_analyzer_model_unknown_provider(self): + cfg = self._base() + cfg.analyzer_model = "acme/gpt-4o" + with pytest.raises(ConfigError, match="unsupported provider"): + cfg.validate() + + def test_analyzer_max_iterations_zero(self): + cfg = self._base() + cfg.analyzer_max_iterations = 0 + with pytest.raises(ConfigError, match="analyzer_max_iterations"): + cfg.validate() + + def test_analyzer_timeout_s_zero(self): + cfg = self._base() + cfg.analyzer_timeout_s = 0 + with pytest.raises(ConfigError, match="analyzer_timeout_s"): + cfg.validate() + def test_invalid_output_format(self): cfg = self._base() cfg.output_format = "json" diff --git a/test/auto_memory/test_orchestrator.py b/test/auto_memory/test_orchestrator.py index 737a9a0..d3db31b 100644 --- a/test/auto_memory/test_orchestrator.py +++ b/test/auto_memory/test_orchestrator.py @@ -572,4 +572,7 @@ def test_delegates_to_module_function(self, tmp_path): callback_results=[cb_result], candidate_path=tmp_path / "cand", iteration_idx=0, + analyzer_model=config.analyzer_model, + analyzer_max_iterations=config.analyzer_max_iterations, + analyzer_timeout_s=config.analyzer_timeout_s, ) From 1baf4a2d0ca0dd6e045e8e5f5fb8adbd49200cda Mon Sep 17 00:00:00 2001 From: Kavya Sree Kaitepalli Date: Thu, 9 Jul 2026 11:10:37 +0000 Subject: [PATCH 3/3] handle whitespace and None results in analyze_failure function --- src/microbots/auto_memory/analyzer.py | 5 +++-- test/auto_memory/test_analyzer.py | 28 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/microbots/auto_memory/analyzer.py b/src/microbots/auto_memory/analyzer.py index 57cde1f..8f2be65 100644 --- a/src/microbots/auto_memory/analyzer.py +++ b/src/microbots/auto_memory/analyzer.py @@ -109,12 +109,13 @@ def analyze_failure( finally: combined_log.unlink(missing_ok=True) - if bot_result.status and bot_result.result: + result = (bot_result.result or "").strip() + if bot_result.status and result: # The LLM produces one narrative diagnosis, not a list of causes; # store it as the summary and leave root_causes empty. return Feedback( iteration_idx=iteration_idx, - summary=bot_result.result.strip(), + summary=result, validator_failures=validator_failures, ) diff --git a/test/auto_memory/test_analyzer.py b/test/auto_memory/test_analyzer.py index 78882c1..59bc1d4 100644 --- a/test/auto_memory/test_analyzer.py +++ b/test/auto_memory/test_analyzer.py @@ -321,6 +321,34 @@ def test_empty_result_treated_as_no_result(self, tmp_path): ) assert any("did not produce a result" in c for c in fb.root_causes) + def test_whitespace_only_result_treated_as_no_result(self, tmp_path): + # status=True but result is whitespace only → strips to "" and must + # take the "did not produce a result" path (not silently produce an + # empty summary). + patcher, _ = _patched_bot(status=True, result=" \n\t ", error=None) + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert fb.summary != "" + assert "1 of 1" in fb.summary + assert any("did not produce a result" in c for c in fb.root_causes) + + def test_none_result_treated_as_no_result(self, tmp_path): + # status=True but result=None must not crash on .strip(). + patcher, _ = _patched_bot(status=True, result=None, error=None) + with patcher: + fb = analyze_failure( + [_result(tmp_path, "tests", passed=False, return_code=1)], + tmp_path / "cand", + iteration_idx=0, + **_ANALYZER_KWARGS, + ) + assert any("did not produce a result" in c for c in fb.root_causes) + # --------------------------------------------------------------------------- # Bot invocation itself raised