From 72e626c69cc01c86d513d4bfcb35ed010e08072a Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 28 Jul 2026 04:04:06 -0400 Subject: [PATCH 1/3] fix: preserve agent and readiness failures --- openadapt_evals/agents/api_agent.py | 10 +++- openadapt_evals/agents/base.py | 7 ++- openadapt_evals/agents/baseline_agent.py | 8 +-- .../agents/claude_computer_use_agent.py | 7 ++- .../agents/planner_grounder_agent.py | 32 +++++++--- openadapt_evals/agents/policy_agent.py | 2 +- openadapt_evals/agents/qwen3vl_agent.py | 10 ++-- openadapt_evals/agents/smol_agent.py | 10 ++-- scripts/run_dc_eval.py | 40 ++++++++----- tests/test_agent_failure_contract.py | 57 ++++++++++++++++++ tests/test_claude_computer_use_agent.py | 6 +- tests/test_planner_grounder_agent.py | 10 ++-- tests/test_qwen3vl_agent.py | 14 ++--- tests/test_run_dc_eval_readiness.py | 58 +++++++++++++++++++ tests/test_runner.py | 6 +- tests/test_smol_agent.py | 4 +- 16 files changed, 216 insertions(+), 65 deletions(-) create mode 100644 tests/test_agent_failure_contract.py create mode 100644 tests/test_run_dc_eval_readiness.py diff --git a/openadapt_evals/agents/api_agent.py b/openadapt_evals/agents/api_agent.py index f3f2f5f..780acdd 100644 --- a/openadapt_evals/agents/api_agent.py +++ b/openadapt_evals/agents/api_agent.py @@ -400,13 +400,17 @@ def act( self._last_step_logs = logs # Parse response into BenchmarkAction - if actions and actions[0] in ("DONE", "FAIL", "WAIT"): - return BenchmarkAction(type="done", raw_action={"waa_action": actions[0]}) + if actions and actions[0] == "DONE": + return BenchmarkAction(type="done", raw_action={"waa_action": "DONE"}) + if actions and actions[0] == "FAIL": + return BenchmarkAction(type="error", raw_action={"waa_action": "FAIL"}) + if actions and actions[0] == "WAIT": + return BenchmarkAction(type="wait", raw_action={"waa_action": "WAIT"}) if actions and actions[0].startswith("computer."): return self._parse_computer_action(actions[0], observation) - return BenchmarkAction(type="done", raw_action={"error": "Could not parse action"}) + return BenchmarkAction(type="error", raw_action={"error": "Could not parse action"}) def predict(self, instruction: str, obs: dict) -> tuple: """WAA-compatible interface: Predict the next action based on observation. diff --git a/openadapt_evals/agents/base.py b/openadapt_evals/agents/base.py index 8260ae0..bb25e77 100644 --- a/openadapt_evals/agents/base.py +++ b/openadapt_evals/agents/base.py @@ -155,9 +155,10 @@ def parse_action_response( break if not action_line: - # Could not parse action, return done + # Parsing did not produce an agent decision. Keep that distinct from + # the model explicitly issuing DONE(). raw_action["parse_error"] = "No action pattern found" - return BenchmarkAction(type="done", raw_action=raw_action) + return BenchmarkAction(type="error", raw_action=raw_action) # Parse CLICK action click_match = re.match( @@ -292,4 +293,4 @@ def parse_action_response( # Unknown action format raw_action["parse_error"] = f"Unknown action format: {action_line}" - return BenchmarkAction(type="done", raw_action=raw_action) + return BenchmarkAction(type="error", raw_action=raw_action) diff --git a/openadapt_evals/agents/baseline_agent.py b/openadapt_evals/agents/baseline_agent.py index 177ee20..1720a11 100644 --- a/openadapt_evals/agents/baseline_agent.py +++ b/openadapt_evals/agents/baseline_agent.py @@ -143,7 +143,7 @@ def act( screenshot = self._observation_to_image(observation) if screenshot is None: logger.warning("No screenshot in observation") - return BenchmarkAction(type="done", raw_action={"error": "No screenshot"}) + return BenchmarkAction(type="error", raw_action={"error": "No screenshot"}) # Format accessibility tree a11y_tree = self._format_a11y_tree(observation.accessibility_tree) @@ -161,7 +161,7 @@ def act( ) except Exception as e: logger.error(f"Adapter predict failed: {e}") - return BenchmarkAction(type="done", raw_action={"error": str(e)}) + return BenchmarkAction(type="error", raw_action={"error": str(e)}) # Convert ParsedAction to BenchmarkAction return self._convert_action(parsed, observation) @@ -245,7 +245,7 @@ def _convert_action( raw_action=raw_action, ) - return BenchmarkAction(type="done", raw_action={**raw_action, "error": "Missing coords"}) + return BenchmarkAction(type="error", raw_action={**raw_action, "error": "Missing coords"}) elif action_type == "type": return BenchmarkAction( @@ -273,7 +273,7 @@ def _convert_action( else: return BenchmarkAction( - type="done", + type="error", raw_action={**raw_action, "error": f"Unknown action: {action_type}"}, ) diff --git a/openadapt_evals/agents/claude_computer_use_agent.py b/openadapt_evals/agents/claude_computer_use_agent.py index 86bb62e..bc08593 100644 --- a/openadapt_evals/agents/claude_computer_use_agent.py +++ b/openadapt_evals/agents/claude_computer_use_agent.py @@ -983,12 +983,15 @@ def _map_action( f"'{action_type}' reached _map_action (should be handled by " "act() loop). Treating as no-op." ) - return BenchmarkAction(type="done", raw_action=raw) + return BenchmarkAction( + type="error", + raw_action={"error": f"Unexpected internal action: {action_type}", **raw}, + ) # Unknown action logger.warning(f"Unknown computer_use action: {action_type}") return BenchmarkAction( - type="done", + type="error", raw_action={"error": f"Unknown action: {action_type}", **raw}, ) diff --git a/openadapt_evals/agents/planner_grounder_agent.py b/openadapt_evals/agents/planner_grounder_agent.py index a925b81..def5eb3 100644 --- a/openadapt_evals/agents/planner_grounder_agent.py +++ b/openadapt_evals/agents/planner_grounder_agent.py @@ -378,7 +378,7 @@ def act( if decision == "FAIL": self._action_history.append("FAIL()") return BenchmarkAction( - type="done", + type="error", raw_action={ "planner_output": planner_output, "source": "planner", @@ -387,10 +387,10 @@ def act( ) if not instruction and not action_type: - logger.warning("Planner returned empty instruction, treating as DONE") - self._action_history.append("DONE() [empty instruction]") + logger.warning("Planner returned an empty instruction") + self._action_history.append("ERROR() [empty instruction]") return BenchmarkAction( - type="done", + type="error", raw_action={ "planner_output": planner_output, "parse_error": "empty_instruction", @@ -831,7 +831,11 @@ def _call_grounder( if action.type != "done": return action - logger.warning("Grounder retry also failed, returning done") + logger.warning("Grounder retry also failed") + return BenchmarkAction( + type="error", + raw_action={"parse_error": "grounder output was not actionable"}, + ) return action @@ -892,7 +896,10 @@ def _call_grounder_http( raw = resp.json()["choices"][0]["message"]["content"] except Exception as exc: logger.error("HTTP grounder call failed: %s", exc) - return BenchmarkAction(type="done") + return BenchmarkAction( + type="error", + raw_action={"error": f"HTTP grounder call failed: {exc}"}, + ) logger.info("HTTP grounder raw output: %s", raw[:200]) @@ -932,7 +939,13 @@ def _parse_bbox_to_action(raw: str) -> BenchmarkAction: if not match: # Last resort: try parse_action_json from openadapt_evals.training.trl_rollout import parse_action_json - return parse_action_json(raw) + action = parse_action_json(raw) + if action.type == "done": + return BenchmarkAction( + type="error", + raw_action={"parse_error": "grounder output was not actionable"}, + ) + return action nums = [float(x) for x in match.groups() if x is not None] @@ -944,7 +957,10 @@ def _parse_bbox_to_action(raw: str) -> BenchmarkAction: x, y = nums[0], nums[1] else: logger.warning("Unexpected number count in bbox: %s", nums) - return BenchmarkAction(type="done") + return BenchmarkAction( + type="error", + raw_action={"parse_error": f"unexpected bbox values: {nums}"}, + ) # Normalize coordinates if x > 1 and y > 1: diff --git a/openadapt_evals/agents/policy_agent.py b/openadapt_evals/agents/policy_agent.py index bb0963d..3986f6a 100644 --- a/openadapt_evals/agents/policy_agent.py +++ b/openadapt_evals/agents/policy_agent.py @@ -122,7 +122,7 @@ def act( return action except Exception as e: logger.error(f"Inference failed: {e}") - return BenchmarkAction(type="done", raw_action={"error": str(e)}) + return BenchmarkAction(type="error", raw_action={"error": str(e)}) def _build_prompt( self, diff --git a/openadapt_evals/agents/qwen3vl_agent.py b/openadapt_evals/agents/qwen3vl_agent.py index 680816b..6d845c8 100644 --- a/openadapt_evals/agents/qwen3vl_agent.py +++ b/openadapt_evals/agents/qwen3vl_agent.py @@ -297,7 +297,7 @@ def parse_qwen_action( raw["action_string"] = action_str else: raw["parse_error"] = "No action found in response" - return BenchmarkAction(type="done", raw_action=raw) + return BenchmarkAction(type="error", raw_action=raw) # --- finished() --- if _RE_FINISHED.search(action_str): @@ -382,7 +382,7 @@ def parse_qwen_action( type="key", key=keys[-1], modifiers=keys[:-1], raw_action=raw ) raw["parse_error"] = "Empty keys list in press()" - return BenchmarkAction(type="done", raw_action=raw) + return BenchmarkAction(type="error", raw_action=raw) # --- scroll(direction="", amount=) --- m = _RE_SCROLL.search(action_str) @@ -416,7 +416,7 @@ def parse_qwen_action( # Unknown action format raw["parse_error"] = f"Unknown action format: {action_str}" - return BenchmarkAction(type="done", raw_action=raw) + return BenchmarkAction(type="error", raw_action=raw) # --------------------------------------------------------------------------- @@ -631,7 +631,7 @@ def act( if image is None: logger.error("No screenshot available in observation") return BenchmarkAction( - type="done", raw_action={"error": "no_screenshot"} + type="error", raw_action={"error": "no_screenshot"} ) # Build user content (aligned with convert_demos training format) @@ -647,7 +647,7 @@ def act( except Exception as e: logger.error(f"Inference failed: {e}") return BenchmarkAction( - type="done", raw_action={"error": f"inference_failed: {e}"} + type="error", raw_action={"error": f"inference_failed: {e}"} ) logger.info(f"Step {self._step_count} raw response: {response_text!r}") diff --git a/openadapt_evals/agents/smol_agent.py b/openadapt_evals/agents/smol_agent.py index 74f0982..2cc4eee 100644 --- a/openadapt_evals/agents/smol_agent.py +++ b/openadapt_evals/agents/smol_agent.py @@ -176,7 +176,7 @@ def parse_smol_action( raw["action_string"] = action_str else: raw["parse_error"] = "No action found in response" - return BenchmarkAction(type="done", raw_action=raw) + return BenchmarkAction(type="error", raw_action=raw) # --- final_answer('...') → done --- m = _RE_FINAL_ANSWER.search(action_str) @@ -228,7 +228,7 @@ def parse_smol_action( type="key", key=keys[-1], modifiers=keys[:-1], raw_action=raw ) raw["parse_error"] = "Empty keys list in press()" - return BenchmarkAction(type="done", raw_action=raw) + return BenchmarkAction(type="error", raw_action=raw) # --- scroll(direction='...', amount=...) --- m = _RE_SCROLL.search(action_str) @@ -270,7 +270,7 @@ def parse_smol_action( # Unknown action format raw["parse_error"] = f"Unknown action format: {action_str}" - return BenchmarkAction(type="done", raw_action=raw) + return BenchmarkAction(type="error", raw_action=raw) # --------------------------------------------------------------------------- @@ -381,7 +381,7 @@ def act( if image is None: logger.error("No screenshot available in observation") return BenchmarkAction( - type="done", raw_action={"error": "no_screenshot"} + type="error", raw_action={"error": "no_screenshot"} ) user_content = self._build_prompt(task.instruction) @@ -392,7 +392,7 @@ def act( except Exception as e: logger.error(f"Inference failed: {e}") return BenchmarkAction( - type="done", raw_action={"error": f"inference_failed: {e}"} + type="error", raw_action={"error": f"inference_failed: {e}"} ) logger.info(f"Step {self._step_count} raw response: {response_text!r}") diff --git a/scripts/run_dc_eval.py b/scripts/run_dc_eval.py index ac92c79..20112a3 100644 --- a/scripts/run_dc_eval.py +++ b/scripts/run_dc_eval.py @@ -68,19 +68,31 @@ def _dismiss_lock_screen(server: str) -> bool: Returns True if lock screen was dismissed or wasn't present. """ - try: - # Check for LogonUI.exe process + def lock_state() -> bool | None: + """Return True when locked, False when unlocked, or None if unknown.""" resp = requests.post( f"{server}/execute", json={"command": 'powershell -Command "(Get-Process LogonUI -ErrorAction SilentlyContinue) -ne $null"'}, timeout=10, ) if not resp.ok: - return True # Can't check, assume OK + print(f" Lock screen check failed: HTTP {resp.status_code}") + return None output = resp.json().get("output", "").strip() - if output != "True": - return True # Not locked + if output == "True": + return True + if output == "False": + return False + print(f" Lock screen check returned an unreadable state: {output!r}") + return None + + try: + state = lock_state() + if state is None: + return False + if state is False: + return True print(" Lock screen detected (LogonUI.exe running), dismissing...") @@ -91,16 +103,19 @@ def _dismiss_lock_screen(server: str) -> bool: timeout=15, ) if resp.ok: - print(" Lock screen dismissed, waiting for desktop...") + print(" Unlock input sent, waiting for desktop...") time.sleep(5) # Wait for desktop to load - return True + unlocked = lock_state() is False + if not unlocked: + print(" Lock screen remained present or could not be rechecked") + return unlocked else: print(f" Failed to dismiss lock screen: {resp.text}") return False except Exception as e: print(f" Lock screen check error: {e}") - return True # Don't block on check failures + return False def _probe(server: str, timeout: int = 10) -> bool: @@ -165,8 +180,7 @@ def ensure_waa_ready( """ # Step 1: Quick probe if _probe(server) and (evaluate_url is None or _probe(evaluate_url)): - _dismiss_lock_screen(server) - return True + return _dismiss_lock_screen(server) # Step 2: Reconnect tunnel print(" WAA unreachable, reconnecting tunnel...") @@ -176,8 +190,7 @@ def ensure_waa_ready( time.sleep(3) if _probe(server) and (evaluate_url is None or _probe(evaluate_url)): print(" Tunnel reconnected, WAA ready!") - _dismiss_lock_screen(server) - return True + return _dismiss_lock_screen(server) # Step 3: Tunnel up but WAA not responding → container restart print(" Tunnel OK but WAA server dead, restarting container...") @@ -202,8 +215,7 @@ def ensure_waa_ready( last_print = elapsed if _probe(server, timeout=10) and (evaluate_url is None or _probe(evaluate_url, timeout=10)): print(f" WAA ready after {elapsed}s!") - _dismiss_lock_screen(server) - return True + return _dismiss_lock_screen(server) time.sleep(10) print(f" TIMEOUT: WAA not ready after {max_wait}s") diff --git a/tests/test_agent_failure_contract.py b/tests/test_agent_failure_contract.py new file mode 100644 index 0000000..b7071ae --- /dev/null +++ b/tests/test_agent_failure_contract.py @@ -0,0 +1,57 @@ +"""Agent failures must remain distinct from a model-issued completion.""" + +from __future__ import annotations + +from types import MethodType + +from openadapt_types import BenchmarkObservation, BenchmarkTask + +from openadapt_evals.agents.api_agent import ApiAgent +from openadapt_evals.agents.base import parse_action_response +from openadapt_evals.agents.baseline_agent import BaselineAgent +from openadapt_evals.agents.policy_agent import PolicyAgent +from openadapt_evals.agents.qwen3vl_agent import parse_qwen_action +from openadapt_evals.agents.smol_agent import parse_smol_action + + +def _task(): + return BenchmarkTask(task_id="t", instruction="do it", domain="desktop") + + +def test_parse_failures_are_error_actions(): + assert parse_action_response("not an action").type == "error" + assert parse_qwen_action("not an action").type == "error" + assert parse_smol_action("not an action").type == "error" + + +def test_api_agent_keeps_terminal_decisions_distinct(): + agent = ApiAgent.__new__(ApiAgent) + agent.predict = MethodType( + lambda self, instruction, obs: ("", ["FAIL"], {}, {}), agent + ) + assert agent.act(BenchmarkObservation(), _task()).type == "error" + + agent.predict = MethodType( + lambda self, instruction, obs: ("", ["WAIT"], {}, {}), agent + ) + assert agent.act(BenchmarkObservation(), _task()).type == "wait" + + agent.predict = MethodType( + lambda self, instruction, obs: ("", ["# parse failed"], {}, {}), agent + ) + assert agent.act(BenchmarkObservation(), _task()).type == "error" + + +def test_missing_observation_and_inference_failure_are_errors(monkeypatch): + baseline = BaselineAgent.__new__(BaselineAgent) + baseline._step_count = 0 + assert baseline.act(BenchmarkObservation(), _task()).type == "error" + + policy = PolicyAgent() + monkeypatch.setattr(policy, "_load_model", lambda: None) + monkeypatch.setattr( + policy, + "_run_inference", + lambda observation, prompt: (_ for _ in ()).throw(RuntimeError("offline")), + ) + assert policy.act(BenchmarkObservation(), _task()).type == "error" diff --git a/tests/test_claude_computer_use_agent.py b/tests/test_claude_computer_use_agent.py index 889bf8b..29c4a24 100644 --- a/tests/test_claude_computer_use_agent.py +++ b/tests/test_claude_computer_use_agent.py @@ -456,8 +456,8 @@ def test_api_error_returns_error(self, agent, mock_anthropic_client): assert action.raw_action["reason"] == "api_call_failed" assert action.raw_action["error_type"] == "infrastructure" - def test_unknown_action_returns_done(self, agent, mock_anthropic_client): - """Unknown action type returns done.""" + def test_unknown_action_returns_error(self, agent, mock_anthropic_client): + """An unknown tool action is not task completion.""" response = create_mock_response( create_tool_use_block("unknown_action_xyz") ) @@ -465,7 +465,7 @@ def test_unknown_action_returns_done(self, agent, mock_anthropic_client): action = agent.act(make_observation(), make_task()) - assert action.type == "done" + assert action.type == "error" def test_key_with_multiple_modifiers(self, agent, mock_anthropic_client): """Key with multiple modifiers splits correctly.""" diff --git a/tests/test_planner_grounder_agent.py b/tests/test_planner_grounder_agent.py index bc1233a..425e5a8 100644 --- a/tests/test_planner_grounder_agent.py +++ b/tests/test_planner_grounder_agent.py @@ -229,7 +229,7 @@ def test_vlm_planner_done(self, mock_extract, mock_vlm, observation, task): @patch("openadapt_evals.vlm.vlm_call") @patch("openadapt_evals.vlm.extract_json") def test_vlm_planner_fail(self, mock_extract, mock_vlm, observation, task): - """VLM planner outputs FAIL, agent returns done with fail reason.""" + """VLM planner FAIL stays distinct from DONE.""" mock_vlm.return_value = '{"decision": "FAIL"}' mock_extract.return_value = { "decision": "FAIL", @@ -245,7 +245,7 @@ def test_vlm_planner_fail(self, mock_extract, mock_vlm, observation, task): ) action = agent.act(observation, task) - assert action.type == "done" + assert action.type == "error" assert "fail_reason" in action.raw_action @patch("openadapt_evals.vlm.vlm_call") @@ -296,10 +296,10 @@ def test_grounder_retries_on_parse_failure( @patch("openadapt_evals.vlm.vlm_call") @patch("openadapt_evals.training.trl_rollout.parse_action_json") - def test_grounder_returns_done_after_both_fail( + def test_grounder_returns_error_after_both_fail( self, mock_parse, mock_vlm, observation, task ): - """Grounder returns done when both attempts fail to parse.""" + """Two parse failures do not become task completion.""" mock_parse.return_value = BenchmarkAction(type="done") mock_vlm.return_value = "unparseable gibberish" @@ -310,7 +310,7 @@ def test_grounder_returns_done_after_both_fail( ) action = agent.act(observation, task) - assert action.type == "done" + assert action.type == "error" # -- Tests: HTTP grounder ---------------------------------------------------- diff --git a/tests/test_qwen3vl_agent.py b/tests/test_qwen3vl_agent.py index fbc2045..cda8d53 100644 --- a/tests/test_qwen3vl_agent.py +++ b/tests/test_qwen3vl_agent.py @@ -166,12 +166,12 @@ def test_finished(self): def test_no_action_found(self): action = parse_qwen_action("I don't know what to do") - assert action.type == "done" + assert action.type == "error" assert "parse_error" in action.raw_action def test_empty_response(self): action = parse_qwen_action("") - assert action.type == "done" + assert action.type == "error" assert "parse_error" in action.raw_action def test_viewport_stored_in_raw_action(self): @@ -223,9 +223,9 @@ def test_no_think_block(self): assert "thinking" not in action.raw_action def test_think_block_only_no_action(self): - """If think block is present but no action follows, return done.""" + """Reasoning without an action is a parse error.""" action = parse_qwen_action("I'm not sure what to do") - assert action.type == "done" + assert action.type == "error" def test_think_with_finished(self): response = ( @@ -530,9 +530,9 @@ def test_left_click_alias(self): assert abs(action.x - 0.5) < 1e-6 def test_press_empty_keys(self): - """press(keys=[]) with empty list should return done.""" + """An empty key list is a parse error.""" action = parse_qwen_action("press(keys=[])") - assert action.type == "done" + assert action.type == "error" assert "parse_error" in action.raw_action def test_type_empty_string(self): @@ -549,7 +549,7 @@ def test_response_preserves_raw(self): def test_whitespace_only_response(self): action = parse_qwen_action(" \n \t ") - assert action.type == "done" + assert action.type == "error" def test_multiline_think_then_action(self): """Realistic model output with thinking then action.""" diff --git a/tests/test_run_dc_eval_readiness.py b/tests/test_run_dc_eval_readiness.py new file mode 100644 index 0000000..da4a9d1 --- /dev/null +++ b/tests/test_run_dc_eval_readiness.py @@ -0,0 +1,58 @@ +"""WAA readiness must include a verified unlocked desktop.""" + +from __future__ import annotations + +import importlib.util +from pathlib import Path +from types import SimpleNamespace + +_SPEC = importlib.util.spec_from_file_location( + "run_dc_eval", Path(__file__).parents[1] / "scripts" / "run_dc_eval.py" +) +assert _SPEC and _SPEC.loader +run_dc_eval = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(run_dc_eval) + + +def test_lock_state_query_failure_is_not_ready(monkeypatch): + monkeypatch.setattr( + run_dc_eval.requests, + "post", + lambda *args, **kwargs: SimpleNamespace(ok=False, status_code=503, text="unavailable"), + ) + + assert run_dc_eval._dismiss_lock_screen("http://waa") is False + + +def test_unreadable_lock_state_is_not_ready(monkeypatch): + monkeypatch.setattr( + run_dc_eval.requests, + "post", + lambda *args, **kwargs: SimpleNamespace(ok=True, status_code=200, text="", json=lambda: {}), + ) + + assert run_dc_eval._dismiss_lock_screen("http://waa") is False + + +def test_unlock_must_be_confirmed(monkeypatch): + responses = iter( + [ + SimpleNamespace(ok=True, status_code=200, text="", json=lambda: {"output": "True"}), + SimpleNamespace(ok=True, status_code=200, text="", json=lambda: {}), + SimpleNamespace(ok=True, status_code=200, text="", json=lambda: {"output": "True"}), + ] + ) + monkeypatch.setattr(run_dc_eval.requests, "post", lambda *args, **kwargs: next(responses)) + monkeypatch.setattr(run_dc_eval.time, "sleep", lambda seconds: None) + + assert run_dc_eval._dismiss_lock_screen("http://waa") is False + + +def test_probe_success_still_requires_desktop_readiness(monkeypatch): + monkeypatch.setattr(run_dc_eval, "_probe", lambda *args, **kwargs: True) + monkeypatch.setattr(run_dc_eval, "_dismiss_lock_screen", lambda server: False) + + assert ( + run_dc_eval.ensure_waa_ready("http://waa", "user", "10.0.0.1", evaluate_url="http://eval") + is False + ) diff --git a/tests/test_runner.py b/tests/test_runner.py index 07775a2..d4cb2ad 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -384,11 +384,11 @@ def test_parse_without_action_prefix(self): assert action.type == "click" assert action.target_node_id == "1" - def test_parse_invalid_returns_done(self): - """Test that invalid response returns done action.""" + def test_parse_invalid_returns_error(self): + """An invalid response is not a completion decision.""" response = "I don't know what to do" action = parse_action_response(response) - assert action.type == "done" + assert action.type == "error" def test_coordinate_normalization(self, sample_observation): """Test that pixel coordinates are normalized.""" diff --git a/tests/test_smol_agent.py b/tests/test_smol_agent.py index cdb2729..9165d3c 100644 --- a/tests/test_smol_agent.py +++ b/tests/test_smol_agent.py @@ -173,12 +173,12 @@ def test_open_app(self): def test_unknown_action(self): action = parse_smol_action("some_unknown_action()") - assert action.type == "done" + assert action.type == "error" assert "parse_error" in action.raw_action def test_empty_response(self): action = parse_smol_action("") - assert action.type == "done" + assert action.type == "error" assert "parse_error" in action.raw_action From c5d2cb07b38dca80b442f23a38eb66b4b4cb2c44 Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 28 Jul 2026 04:15:09 -0400 Subject: [PATCH 2/3] fix: make terminal agent failures authoritative --- .../agents/planner_grounder_agent.py | 25 +++++++++- openadapt_evals/benchmarks/runner.py | 17 ++++++- tests/test_planner_grounder_agent.py | 49 +++++++++++++++++++ tests/test_runner.py | 36 ++++++++++++++ 4 files changed, 124 insertions(+), 3 deletions(-) diff --git a/openadapt_evals/agents/planner_grounder_agent.py b/openadapt_evals/agents/planner_grounder_agent.py index def5eb3..211d243 100644 --- a/openadapt_evals/agents/planner_grounder_agent.py +++ b/openadapt_evals/agents/planner_grounder_agent.py @@ -375,14 +375,21 @@ def act( }, ) - if decision == "FAIL": + if decision in ("FAIL", "ERROR"): self._action_history.append("FAIL()") + error = ( + planner_output.get("error") + or reasoning + or "planner reported failure" + ) return BenchmarkAction( type="error", raw_action={ "planner_output": planner_output, "source": "planner", "fail_reason": reasoning, + "error": error, + "error_type": planner_output.get("error_type", "agent"), }, ) @@ -994,6 +1001,22 @@ def _action_to_planner_output(action: BenchmarkAction) -> dict[str, Any]: "reasoning": action.raw_action.get("reasoning", "") if action.raw_action else "", } + if action.type == "error": + raw_action = action.raw_action or {} + error = ( + raw_action.get("reason") + or raw_action.get("error") + or raw_action.get("parse_error") + or "planner reported failure" + ) + return { + "decision": "ERROR", + "instruction": "", + "reasoning": str(error), + "error": str(error), + "error_type": raw_action.get("error_type", "agent"), + } + # Use the action string representation as the instruction. instruction = action_to_string(action) diff --git a/openadapt_evals/benchmarks/runner.py b/openadapt_evals/benchmarks/runner.py index 43e344e..14c396a 100644 --- a/openadapt_evals/benchmarks/runner.py +++ b/openadapt_evals/benchmarks/runner.py @@ -509,10 +509,23 @@ def _run_single_task( logger.info("Evaluating task result") result = adapter.evaluate(task) - # Propagate error_type from agent error action + # An evaluator observes target state, not whether the agent itself + # completed this attempt safely. A pre-existing target state can score + # as successful even after the agent reported a provider or parse + # failure, so the terminal agent error remains authoritative. if action is not None and action.type == "error" and action.raw_action: + error_reason = ( + action.raw_action.get("reason") + or action.raw_action.get("error") + or action.raw_action.get("parse_error") + or action.raw_action.get("fail_reason") + or "agent reported a terminal error" + ) + result.success = False + result.score = 0.0 result.error_type = action.raw_action.get("error_type", "agent") - result.error = result.error or action.raw_action.get("reason") + result.error = str(error_reason) + result.reason = str(error_reason) # Update result with trajectory info result.steps = history if config.save_trajectories else [] diff --git a/tests/test_planner_grounder_agent.py b/tests/test_planner_grounder_agent.py index 425e5a8..8fb2f6b 100644 --- a/tests/test_planner_grounder_agent.py +++ b/tests/test_planner_grounder_agent.py @@ -82,6 +82,22 @@ def reset(self): pass +class MockErrorPlannerAgent: + """Mock planner that reports a terminal provider failure.""" + + def act(self, observation, task, history=None): + return BenchmarkAction( + type="error", + raw_action={ + "error": "provider offline", + "error_type": "infrastructure", + }, + ) + + def reset(self): + pass + + # -- Tests: Agent-based planner + grounder ------------------------------------ @@ -109,6 +125,19 @@ def test_planner_done_returns_done(self, observation, task): assert action.type == "done" assert action.raw_action["source"] == "planner" + def test_planner_error_stops_before_grounding(self, observation, task): + """A planner failure cannot be converted into a grounded action.""" + planner = MockErrorPlannerAgent() + grounder = MagicMock() + + agent = PlannerGrounderAgent(planner=planner, grounder=grounder) + action = agent.act(observation, task) + + assert action.type == "error" + assert action.raw_action["error"] == "provider offline" + assert action.raw_action["error_type"] == "infrastructure" + grounder.act.assert_not_called() + def test_planner_metadata_attached_to_action(self, observation, task): """Planner output dict is attached to the final action's raw_action.""" planner = MockPlannerAgent() @@ -397,6 +426,26 @@ def test_done_action_no_raw_action(self): assert result["decision"] == "DONE" assert result["reasoning"] == "" + def test_error_action_maps_to_fail_decision(self): + """Planner errors remain terminal and retain their diagnostics.""" + action = BenchmarkAction( + type="error", + raw_action={ + "error": "provider offline", + "error_type": "infrastructure", + }, + ) + + result = _action_to_planner_output(action) + + assert result == { + "decision": "ERROR", + "instruction": "", + "reasoning": "provider offline", + "error": "provider offline", + "error_type": "infrastructure", + } + # -- Tests: Accessibility tree formatting ------------------------------------- diff --git a/tests/test_runner.py b/tests/test_runner.py index d4cb2ad..ef6dd30 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -555,3 +555,39 @@ def test_stops_on_done_action(self): # Should have only taken 0 steps (done before first step execution) assert result.num_steps == 0 + + def test_agent_error_cannot_be_overwritten_by_successful_evaluation(self): + """A terminal agent error cannot be reported as task success.""" + adapter = WAAMockAdapter(num_tasks=1, domains=["browser"]) + task = adapter.list_tasks()[0] + adapter.evaluate = Mock( + return_value=BenchmarkResult( + task_id=task.task_id, + success=True, + score=1.0, + ) + ) + agent = ScriptedAgent( + [ + BenchmarkAction( + type="error", + raw_action={ + "error": "provider offline", + "error_type": "infrastructure", + }, + ) + ] + ) + config = EvaluationConfig( + verbose=False, + save_execution_traces=False, + enable_live_tracking=False, + ) + + result = _run_single_task(agent, adapter, task, config) + + assert result.success is False + assert result.score == 0.0 + assert result.error == "provider offline" + assert result.reason == "provider offline" + assert result.error_type == "infrastructure" From c1951885a46a59926b4f73e29566c8961570bcf0 Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 28 Jul 2026 04:17:56 -0400 Subject: [PATCH 3/3] test: cover diagnostic-free agent errors --- openadapt_evals/benchmarks/runner.py | 13 +++++++------ tests/test_runner.py | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/openadapt_evals/benchmarks/runner.py b/openadapt_evals/benchmarks/runner.py index 14c396a..29d2310 100644 --- a/openadapt_evals/benchmarks/runner.py +++ b/openadapt_evals/benchmarks/runner.py @@ -513,17 +513,18 @@ def _run_single_task( # completed this attempt safely. A pre-existing target state can score # as successful even after the agent reported a provider or parse # failure, so the terminal agent error remains authoritative. - if action is not None and action.type == "error" and action.raw_action: + if action is not None and action.type == "error": + raw_action = action.raw_action or {} error_reason = ( - action.raw_action.get("reason") - or action.raw_action.get("error") - or action.raw_action.get("parse_error") - or action.raw_action.get("fail_reason") + raw_action.get("reason") + or raw_action.get("error") + or raw_action.get("parse_error") + or raw_action.get("fail_reason") or "agent reported a terminal error" ) result.success = False result.score = 0.0 - result.error_type = action.raw_action.get("error_type", "agent") + result.error_type = raw_action.get("error_type", "agent") result.error = str(error_reason) result.reason = str(error_reason) diff --git a/tests/test_runner.py b/tests/test_runner.py index ef6dd30..c5bb954 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -591,3 +591,29 @@ def test_agent_error_cannot_be_overwritten_by_successful_evaluation(self): assert result.error == "provider offline" assert result.reason == "provider offline" assert result.error_type == "infrastructure" + + def test_agent_error_without_diagnostics_cannot_report_success(self): + """The terminal action type alone is sufficient to refuse success.""" + adapter = WAAMockAdapter(num_tasks=1, domains=["browser"]) + task = adapter.list_tasks()[0] + adapter.evaluate = Mock( + return_value=BenchmarkResult( + task_id=task.task_id, + success=True, + score=1.0, + ) + ) + agent = ScriptedAgent([BenchmarkAction(type="error")]) + config = EvaluationConfig( + verbose=False, + save_execution_traces=False, + enable_live_tracking=False, + ) + + result = _run_single_task(agent, adapter, task, config) + + assert (result.success, result.score, result.error_type) == ( + False, + 0.0, + "agent", + )