Merge pull request #405 from hud-evals/j/fix-gemini-computer

Fix coordinate scaling for gemini

Author: jdchawla29

hud/tools/computer/gemini.py

@@ -228,8 +228,14 @@ async def __call__(
 
         # Helper to finalize ContentResult: rescale if requested and ensure URL metadata
         async def _finalize(
-            result: ContentResult, requested_url: str | None = None
+            result: ContentResult,
+            requested_url: str | None = None,
+            output: str | None = None,
         ) -> list[ContentBlock]:
+            if output is not None and result.error is None:
+                result.output = output
+            elif result.error == "":
+                result.error = "Tool execution failed with no error output"
             if result.base64_image and self.rescale_images:
                 try:
                     result.base64_image = await self._rescale_screenshot(result.base64_image)
@@ -253,14 +259,14 @@ async def _finalize(
                 raise McpError(ErrorData(code=INVALID_PARAMS, message="x and y are required"))
             sx, sy = self._scale_coordinates(x, y)
             result = await self.executor.click(x=sx, y=sy)
-            return await _finalize(result)
+            return await _finalize(result, output=f"Clicked at ({x}, {y})")
 
         elif action == "hover_at":
             if x is None or y is None:
                 raise McpError(ErrorData(code=INVALID_PARAMS, message="x and y are required"))
             sx, sy = self._scale_coordinates(x, y)
             result = await self.executor.move(x=sx, y=sy)
-            return await _finalize(result)
+            return await _finalize(result, output=f"Hovered at ({x}, {y})")
 
         elif action == "type_text_at":
             if x is None or y is None:
@@ -284,7 +290,7 @@ async def _finalize(
 
             # Type (optionally press enter after)
             result = await self.executor.write(text=text, enter_after=bool(press_enter))
-            return await _finalize(result)
+            return await _finalize(result, output=f"Typed text at ({x}, {y})")
 
         elif action == "scroll_document":
             if direction is None:
@@ -317,7 +323,7 @@ async def _finalize(
                     ErrorData(code=INVALID_PARAMS, message=f"Invalid direction: {direction}")
                 )
             result = await self.executor.scroll(scroll_x=scroll_x, scroll_y=scroll_y)
-            return await _finalize(result)
+            return await _finalize(result, output=f"Scrolled document {direction}")
 
         elif action == "scroll_at":
             if direction is None:
@@ -351,7 +357,7 @@ async def _finalize(
                     ErrorData(code=INVALID_PARAMS, message=f"Invalid direction: {direction}")
                 )
             result = await self.executor.scroll(x=sx, y=sy, scroll_x=scroll_x, scroll_y=scroll_y)
-            return await _finalize(result)
+            return await _finalize(result, output=f"Scrolled {direction} at ({x}, {y})")
 
         elif action == "wait_5_seconds":
             result = await self.executor.wait(time=5000)
@@ -419,7 +425,10 @@ async def _finalize(
             )
             path = self._inset_scaled_drag_path(path)
             result = await self.executor.drag(path=path)
-            return await _finalize(result)
+            return await _finalize(
+                result,
+                output=f"Dragged from ({x}, {y}) to ({destination_x}, {destination_y})",
+            )
 
         else:
             raise McpError(ErrorData(code=INVALID_PARAMS, message=f"Unknown action: {action}"))

hud/tools/computer/hud.py

@@ -24,7 +24,7 @@
 
 
 class AgentCoordinate(int):
-    """Carry both execution and model-visible coordinate values."""
+    """Execution pixel coordinate with optional model-coordinate metadata."""
 
     agent_value: int
 
@@ -33,15 +33,6 @@ def __new__(cls, value: int, agent_value: int) -> Self:
         obj.agent_value = agent_value
         return obj
 
-    def __format__(self, format_spec: str) -> str:
-        return format(self.agent_value, format_spec)
-
-    def __str__(self) -> str:
-        return str(int(self))
-
-    def __repr__(self) -> str:
-        return repr(self.agent_value)
-
 
 class HudComputerTool(BaseTool):
     """

hud/tools/computer/tests/test_computer.py

@@ -8,7 +8,7 @@
 from hud.tools.computer.anthropic import AnthropicComputerTool
 from hud.tools.computer.gemini import GeminiComputerTool
 from hud.tools.computer.glm import GLMComputerTool
-from hud.tools.computer.hud import HudComputerTool
+from hud.tools.computer.hud import AgentCoordinate, HudComputerTool
 from hud.tools.computer.openai import OpenAIComputerTool
 from hud.tools.computer.qwen import QwenComputerTool
 from hud.tools.executors.base import BaseExecutor
@@ -36,6 +36,11 @@ async def drag(self, path, pattern=None, hold_keys=None, take_screenshot=True):
         return await super().drag(path, pattern, hold_keys, take_screenshot=False)
 
 
+class EmptyErrorExecutor(BaseExecutor):
+    async def click(self, *args, **kwargs):
+        return ContentResult(error="")
+
+
 @pytest.mark.asyncio
 async def test_hud_computer_screenshot():
     comp = HudComputerTool()
@@ -75,15 +80,42 @@ async def test_anthropic_computer_screenshot():
 
 
 @pytest.mark.asyncio
-async def test_gemini_computer_click_reports_agent_coordinates():
+async def test_gemini_computer_scaling_preserves_model_coordinates():
     comp = GeminiComputerTool()
+    x, y = comp._scale_coordinates(214, 420)
+
+    assert x is not None
+    assert y is not None
+    assert int(x) != 214
+    assert int(y) != 420
+    assert getattr(x, "agent_value") == 214
+    assert getattr(y, "agent_value") == 420
+
+
+@pytest.mark.asyncio
+async def test_gemini_computer_click_reports_model_coordinates():
+    comp = GeminiComputerTool(executor=BaseExecutor())
+
     blocks = await comp(action="click_at", x=214, y=420)
 
     assert any(
-        "(214, 420)" in content.text for content in blocks if isinstance(content, TextContent)
+        "Clicked at (214, 420)" in content.text
+        for content in blocks
+        if isinstance(content, TextContent)
     )
 
 
+@pytest.mark.asyncio
+async def test_gemini_computer_does_not_mask_empty_error():
+    comp = GeminiComputerTool(executor=EmptyErrorExecutor())
+
+    blocks = await comp(action="click_at", x=214, y=420)
+    text = "\n".join(content.text for content in blocks if isinstance(content, TextContent))
+
+    assert "Clicked at (214, 420)" not in text
+    assert "Tool execution failed with no error output" in text
+
+
 @pytest.mark.asyncio
 async def test_anthropic_computer_zoom():
     """Test zoom action on AnthropicComputerTool.
@@ -125,40 +157,44 @@ async def test_anthropic_computer_zoom():
 
 @pytest.mark.asyncio
 async def test_openai_computer_click():
-    comp = OpenAIComputerTool()
+    comp = OpenAIComputerTool(executor=BaseExecutor())
     blocks = await comp(type="click", x=5, y=5)
     assert blocks
-    assert any("(5, 5)" in content.text for content in blocks if isinstance(content, TextContent))
 
 
 @pytest.mark.asyncio
-async def test_anthropic_computer_click_reports_agent_coordinates():
-    comp = AnthropicComputerTool()
-    blocks = await comp(action="left_click", coordinate=[123, 456], text=None)
+async def test_anthropic_computer_scaling_preserves_agent_coordinates():
+    comp = AnthropicComputerTool(executor=BaseExecutor())
+    x, y = comp._scale_coordinates(123, 456)
 
-    assert any(
-        "(123, 456)" in content.text for content in blocks if isinstance(content, TextContent)
-    )
+    assert x is not None
+    assert y is not None
+    assert getattr(x, "agent_value") == 123
+    assert getattr(y, "agent_value") == 456
 
 
 @pytest.mark.asyncio
-async def test_qwen_computer_click_reports_agent_coordinates():
-    comp = QwenComputerTool()
-    blocks = await comp(action="left_click", coordinate=[123, 456])
+async def test_qwen_computer_scaling_preserves_agent_coordinates():
+    comp = QwenComputerTool(executor=BaseExecutor())
+    x, y = comp._scale_coordinates(123, 456)
 
-    assert any(
-        "(123, 456)" in content.text for content in blocks if isinstance(content, TextContent)
-    )
+    assert x is not None
+    assert y is not None
+    assert getattr(x, "agent_value") == 123
+    assert getattr(y, "agent_value") == 456
 
 
 @pytest.mark.asyncio
-async def test_glm_computer_click_reports_agent_coordinates():
-    comp = GLMComputerTool()
-    blocks = await comp(action="left_click", start_box="[123,456]")
+async def test_glm_computer_scaling_preserves_model_coordinates():
+    comp = GLMComputerTool(executor=BaseExecutor())
+    x, y = comp._scale_coordinates(123, 456)
 
-    assert any(
-        "(123, 456)" in content.text for content in blocks if isinstance(content, TextContent)
-    )
+    assert x is not None
+    assert y is not None
+    assert int(x) != 123
+    assert int(y) != 456
+    assert getattr(x, "agent_value") == 123
+    assert getattr(y, "agent_value") == 456
 
 
 def test_normalized_coordinate_max_stays_in_display_bounds():
@@ -217,6 +253,28 @@ async def test_xdo_drag_executes_interpolated_mouse_moves():
     assert mouse_moves[-1] == "mousemove 120 0"
 
 
+@pytest.mark.asyncio
+async def test_xdo_commands_use_execution_pixels_for_agent_coordinates():
+    executor = RecordingXDOExecutor()
+
+    await executor.click(x=AgentCoordinate(309, 214), y=AgentCoordinate(396, 420))
+
+    assert executor.commands[-1] == "mousemove 309 396 click 1"
+
+
+@pytest.mark.asyncio
+async def test_xdo_nonzero_empty_stderr_surfaces_error(monkeypatch):
+    async def fake_run(command: str):
+        return 1, "", ""
+
+    monkeypatch.setattr("hud.tools.executors.xdo.run", fake_run)
+    executor = XDOExecutor()
+
+    result = await executor.execute("mousemove 1 2", take_screenshot=False)
+
+    assert result.error == "Command failed with exit code 1"
+
+
 class TestHudComputerToolExtended:
     """Extended tests for HudComputerTool covering edge cases and platform logic."""
 

hud/tools/executors/xdo.py

@@ -5,11 +5,13 @@
 import logging
 import os
 import shlex
-from pathlib import Path
+from contextlib import suppress
 from tempfile import gettempdir
 from typing import Literal
 from uuid import uuid4
 
+from anyio import Path
+
 from hud.tools.types import ContentResult
 from hud.tools.utils import run
 
@@ -141,9 +143,14 @@ async def execute(self, command: str, take_screenshot: bool = True) -> ContentRe
         # Execute command
         returncode, stdout, stderr = await run(full_command)
 
+        error = None
+        if returncode != 0:
+            error = stderr or f"Command failed with exit code {returncode}"
+
         # Prepare result
         result = ContentResult(
-            output=stdout if stdout else None, error=stderr if stderr or returncode != 0 else None
+            output=stdout if stdout else None,
+            error=error,
         )
 
         # Take screenshot if requested
@@ -167,7 +174,7 @@ async def screenshot(self) -> str | None:
         # Real screenshot using scrot
         if OUTPUT_DIR:
             output_dir = Path(OUTPUT_DIR)
-            output_dir.mkdir(parents=True, exist_ok=True)
+            await output_dir.mkdir(parents=True, exist_ok=True)
             screenshot_path = output_dir / f"screenshot_{uuid4().hex}.png"
         else:
             # Generate a unique path in system temp dir without opening a file
@@ -177,12 +184,13 @@ async def screenshot(self) -> str | None:
 
         returncode, _, _stderr = await run(screenshot_cmd)
 
-        if returncode == 0 and screenshot_path.exists():
+        if returncode == 0 and await screenshot_path.exists():
             try:
-                image_data = screenshot_path.read_bytes()
+                image_data = await screenshot_path.read_bytes()
                 # Remove the file unless user requested persistence via env var
                 if not OUTPUT_DIR:
-                    screenshot_path.unlink(missing_ok=True)
+                    with suppress(FileNotFoundError):
+                        await screenshot_path.unlink()
                 return base64.b64encode(image_data).decode()
             except Exception:
                 return None