diff --git a/src/fara/agents/fara/fara15_agent.py b/src/fara/agents/fara/fara15_agent.py index c7c1632..64b168f 100644 --- a/src/fara/agents/fara/fara15_agent.py +++ b/src/fara/agents/fara/fara15_agent.py @@ -30,7 +30,7 @@ ) from ._prompts import get_computer_use_system_prompt -from ...clients.wrapper import ChatCompletionClient +from ...clients.wrapper import ChatCompletionClient, extract_message_text from ...clients.create_utils import create_client_from_config from ...clients.messages import ( LLMMessage, @@ -103,6 +103,7 @@ class Fara15AgentConfig(AgentConfig): captcha_timeout_limit: int = 2 raise_on_captcha_timeout: bool = True terminate_on_parse_error: bool = False + max_parse_retries: int = 2 image_token_estimate: int = 1500 image_budget_token_cap: int = 0 @@ -438,33 +439,77 @@ def _get_system_message( return system_message, scaled_screenshot def _parse_thoughts_and_action(self, message: str) -> Tuple[str, Dict[str, Any]]: + """Parse a model response without depending on exact whitespace. + + The training format uses ```` tags, but OpenAI-compatible + servers sometimes remove a newline, add a Markdown fence, or return the + JSON object without the tags. All of those forms are accepted here. + """ try: - tmp = message.split("\n") - thoughts = tmp[0].strip() - action_text = tmp[1].split("\n")[0] + if not isinstance(message, str) or not message.strip(): + raise ValueError("the model returned an empty response") + + opening_tag = re.search(r"]*>", message, re.IGNORECASE) + if opening_tag: + thoughts = message[: opening_tag.start()].strip() + action_text = message[opening_tag.end() :] + closing_tag = re.search(r"", action_text, re.IGNORECASE) + if closing_tag: + action_text = action_text[: closing_tag.start()] + else: + # Be liberal with servers that return a bare/fenced object. + object_start = message.find("{") + if object_start < 0: + raise ValueError("no tag or JSON object was found") + thoughts = message[:object_start].strip() + action_text = message[object_start:] + + action_text = action_text.strip() + if action_text.startswith("```"): + action_text = re.sub( + r"^```(?:json|python)?\s*", + "", + action_text, + count=1, + flags=re.IGNORECASE, + ) + action_text = re.sub(r"\s*```\s*$", "", action_text, count=1) + try: - action = json.loads(action_text) - except json.decoder.JSONDecodeError: - self.logger.error(f"Invalid action text: {action_text}") - action = ast.literal_eval(action_text) + action, _ = json.JSONDecoder().raw_decode(action_text.lstrip()) + except json.decoder.JSONDecodeError as json_error: + try: + action = ast.literal_eval(action_text) + except (SyntaxError, ValueError) as literal_error: + raise ValueError( + f"tool call is not valid JSON: {json_error.msg}" + ) from literal_error + + if not isinstance(action, dict): + raise ValueError("tool call must be a JSON object") + if not isinstance(action.get("name"), str) or not action["name"]: + raise ValueError("tool call is missing a non-empty 'name'") + if not isinstance(action.get("arguments"), dict): + raise ValueError("tool call is missing an 'arguments' object") + if not isinstance(action["arguments"].get("action"), str): + raise ValueError("tool call arguments are missing a string 'action'") return thoughts, action - except Exception as e: - self.logger.error( - f"Error parsing thoughts and action: {message}", exc_info=True - ) + except (TypeError, ValueError) as error: if self.config.terminate_on_parse_error: self.logger.warning( - "terminate_on_parse_error=true: ending trajectory with raw " - "model response as final answer" + "Could not parse model response (%s); ending trajectory because " + "terminate_on_parse_error=true", + error, ) - return message.strip(), { + raw_message = message.strip() if isinstance(message, str) else "" + return raw_message, { "name": "computer_use", "arguments": { "action": "terminate", - "answer": message.strip(), + "answer": raw_message, }, } - raise e + raise ValueError(f"Could not parse model tool call: {error}") from error def convert_resized_coords_to_original( self, coords: List[float], rsz_w: int, rsz_h: int, og_w: int, og_h: int @@ -523,7 +568,7 @@ async def _make_model_call( messages=history, extra_create_args=extra_create_args or {}, ) - return result.content.content + return extract_message_text(result.content) def remove_screenshot_from_message(self, msg: LLMMessage) -> LLMMessage | None: """Remove the screenshot from the message content.""" @@ -667,10 +712,36 @@ async def _generate_model_call( call_args: dict[str, Any] = {"temperature": 0} if self.config.extra_create_args: call_args.update(self.config.extra_create_args) - message = await self._make_model_call(history, extra_create_args=call_args) + max_attempts = max(1, self.config.max_parse_retries + 1) + for attempt in range(1, max_attempts + 1): + message = await self._make_model_call(history, extra_create_args=call_args) + assistant_message = AssistantMessage(content=message or "") + self._state.chat_history.append(assistant_message) + try: + thoughts, action = self._parse_thoughts_and_action(message) + break + except ValueError as error: + if attempt == max_attempts: + raise ValueError( + f"Model returned an invalid tool call after {max_attempts} " + f"attempts: {error}" + ) from error + self.logger.warning( + "Invalid model tool call (attempt %d/%d): %s; retrying", + attempt, + max_attempts, + error, + ) + correction = UserMessage( + content=( + "Your previous response could not be parsed. Reply with exactly " + "one valid JSON tool call inside and " + "tags, following the provided tool schema." + ) + ) + self._state.chat_history.append(correction) + history.extend([assistant_message, correction]) - self._state.chat_history.append(AssistantMessage(content=message)) - thoughts, action = self._parse_thoughts_and_action(message) action["arguments"]["thoughts"] = thoughts function_call = [FunctionCall(id="dummy", **action)] return function_call, message diff --git a/src/fara/clients/wrapper.py b/src/fara/clients/wrapper.py index edecfa1..bf83a3c 100644 --- a/src/fara/clients/wrapper.py +++ b/src/fara/clients/wrapper.py @@ -7,6 +7,7 @@ (``.content.content`` is the text). """ +import json from typing import Any, Dict, List from openai import AsyncOpenAI @@ -14,6 +15,87 @@ from .messages import CreateResult, LLMMessage, RequestUsage, message_to_openai_format +def _text_value(value: Any) -> str: + """Flatten the text-like content variants used by compatible servers.""" + if isinstance(value, str): + return value + if isinstance(value, list): + parts = [_text_value(item) for item in value] + return "\n".join(part for part in parts if part) + if isinstance(value, dict): + for key in ("text", "content", "value"): + if key in value: + return _text_value(value[key]) + return "" + for attribute in ("text", "content", "value"): + nested = getattr(value, attribute, None) + if nested is not None and nested is not value: + return _text_value(nested) + return "" + + +def extract_message_text(message: Any) -> str: + """Get generated text from an OpenAI or OpenAI-compatible message. + + Some reasoning servers put the entire generated response in a non-standard + reasoning field and leave ``content`` empty. Native function calls are + normalized back into the textual format expected by Fara's trained parser. + """ + if isinstance(message, str): + return message + + values: list[str] = [] + standard_content = _text_value(getattr(message, "content", None)).strip() + + extras = getattr(message, "model_extra", None) or {} + for field_name in ("reasoning_content", "reasoning", "analysis"): + value = getattr(message, field_name, None) + if value is None and isinstance(extras, dict): + value = extras.get(field_name) + text = _text_value(value).strip() + if text and text not in values: + values.append(text) + + if standard_content and standard_content not in values: + values.append(standard_content) + + tool_calls = getattr(message, "tool_calls", None) or [] + for tool_call in tool_calls: + function = getattr(tool_call, "function", None) + if function is None and isinstance(tool_call, dict): + function = tool_call.get("function") + name = ( + function.get("name") + if isinstance(function, dict) + else getattr(function, "name", None) + ) + arguments = ( + function.get("arguments") + if isinstance(function, dict) + else getattr(function, "arguments", None) + ) + if not name: + continue + if isinstance(arguments, str): + try: + arguments = json.loads(arguments) + except json.JSONDecodeError: + # Keep malformed arguments visible to the normal parser, which + # will produce a useful error and trigger its bounded retry. + tool_text = ( + f'{{"name": {json.dumps(name)}, ' + f'"arguments": {arguments}}}' + ) + values.append(tool_text) + continue + tool_text = json.dumps( + {"name": name, "arguments": arguments or {}}, ensure_ascii=False + ) + values.append(f"{tool_text}") + + return "\n".join(values) + + class ChatCompletionClient: """Chat completion client backed by an OpenAI-compatible endpoint.""" diff --git a/tests/test_fara15.py b/tests/test_fara15.py index d59ee91..853d0f9 100644 --- a/tests/test_fara15.py +++ b/tests/test_fara15.py @@ -5,14 +5,18 @@ DataPoint trajectory format. """ +import asyncio import json import logging +from types import SimpleNamespace +import pytest from PIL import Image from fara import DataPoint, Fara15Agent, Task from fara.agents.fara.fara15_agent import ( Fara15AgentConfig, + Fara15AgentState, extract_allowed_actions, ) from fara.agents.fara._prompts import ( @@ -25,6 +29,7 @@ SolverStatus, ToolOutput, ) +from fara.clients.wrapper import extract_message_text BROWSER_ACTIONS = { "key", @@ -108,7 +113,11 @@ def test_system_prompt_byte_identical_to_training(): ensure_ascii=False, ) template = ( - FARA_QWEN35_IDENTITY + "\n\n" + CRITICAL_POINTS_FARA_1_5 + "\n\n" + FN_CALL_FORMAT + FARA_QWEN35_IDENTITY + + "\n\n" + + CRITICAL_POINTS_FARA_1_5 + + "\n\n" + + FN_CALL_FORMAT ) expected = template.format(tool_descs=tool_descs) assert _system_text(_make_agent()) == expected @@ -136,10 +145,106 @@ def test_parse_thoughts_and_action(): assert action["arguments"]["action"] == "left_click" +@pytest.mark.parametrize( + "message", + [ + '{"name":"computer_use","arguments":{"action":"wait"}}', + '\n```json\n{"name":"computer_use","arguments":{"action":"wait"}}\n```\n', + '{"name":"computer_use","arguments":{"action":"wait"}}', + "{'name': 'computer_use', 'arguments': {'action': 'wait'}}", + ], +) +def test_parse_thoughts_and_action_accepts_common_server_variants(message): + agent = _make_agent() + _, action = agent._parse_thoughts_and_action(message) + assert action == {"name": "computer_use", "arguments": {"action": "wait"}} + + +@pytest.mark.parametrize( + ("message", "error_text"), + [ + ("", "empty response"), + ("I forgot to call a tool", "no tag or JSON object"), + ('{"name":"computer_use"}', "'arguments' object"), + ], +) +def test_parse_thoughts_and_action_reports_useful_errors(message, error_text): + agent = _make_agent() + with pytest.raises(ValueError, match=error_text): + agent._parse_thoughts_and_action(message) + + +def test_generate_model_call_retries_an_empty_response(): + agent = _make_agent() + agent._state = Fara15AgentState(mlm_width=1440, mlm_height=900) + responses = iter( + [ + "", + '{"name":"computer_use","arguments":{"action":"wait"}}', + ] + ) + call_count = 0 + + async def fake_model_call(history, extra_create_args=None): + nonlocal call_count + call_count += 1 + return next(responses) + + agent._make_model_call = fake_model_call + function_call, _ = asyncio.run( + agent._generate_model_call( + env=None, + is_first_round=True, + first_screenshot=Image.new("RGB", (1440, 900)), + ) + ) + + assert call_count == 2 + assert function_call[0].arguments["action"] == "wait" + assert any( + "previous response could not be parsed" in message.content + for message in agent._state.chat_history + ) + + +def test_extract_message_text_accepts_reasoning_content(): + message = SimpleNamespace( + content="", + model_extra={ + "reasoning_content": ( + '{"name":"computer_use",' + '"arguments":{"action":"wait"}}' + ) + }, + tool_calls=None, + ) + assert extract_message_text(message).startswith("") + + +def test_extract_message_text_normalizes_native_tool_calls(): + message = SimpleNamespace( + content=None, + model_extra={}, + tool_calls=[ + SimpleNamespace( + function=SimpleNamespace( + name="computer_use", arguments='{"action":"wait"}' + ) + ) + ], + ) + text = extract_message_text(message) + agent = _make_agent() + _, action = agent._parse_thoughts_and_action(text) + assert action["arguments"]["action"] == "wait" + + def test_data_point_roundtrip(tmp_path): dp = DataPoint(task=Task(task_id="t1", instruction="find a hotel")) dp.solver_log.add_observation( - ComputerObservation(screenshot_path="screenshot_1_pre.png", url="https://bing.com") + ComputerObservation( + screenshot_path="screenshot_1_pre.png", url="https://bing.com" + ) ) action = Action( action_name="left_click",