fix(perception): reconcile points/pixels scale and stale overlay screenshot
Tests / Test tests.test_device_config.test_device_config_store_settings_get_set_and_defaults failed
Tests / Test tests.test_device_config.test_device_config_store_settings_get_set_and_defaults failed
Host-agent console showed OCR/UI-tree overlay boxes misaligned with the displayed screenshot. Two independent causes, both confirmed with real task data and pixel-level measurement of a user-provided screenshot: 1. perception/ui_parser.py parses XCUITest UI-tree bounds as iOS logical points, while scene_builder.py's Scene.width/height (via infer_png_size) and OCR bounds are in screenshot pixels, never reconciled (2.0x on Retina devices). build_scene() now detects the scale from the first x==0,y==0 UI element and rescales OCR bounds down to points-space, reporting Scene.width/height in points too. No-op for Android, where UiAutomator2 bounds already match pixels 1:1. This also fixes tap() landing at the wrong location for OCR-matched text, and lets the IOU fusion between UI-tree and OCR elements actually fire on iOS. 2. runtime/task.py captured `scene` (OCR/UI-tree data) before the LLM planning call, but re-captured `before_screenshot` for each step afterward - a real time gap during which on-screen content (e.g. a keyboard) could shift, producing a directional drift between the overlay and the displayed image. The first step of each plan batch now reuses the screenshot already taken for planning instead of capturing a new one; later steps in a multi-step batch still take a fresh capture (left unresolved, scoped out by request). Regression tests added for both the scale reconciliation (using real 828x1792 vs 414x896 numbers) and the screenshot reuse behavior.
This commit is contained in:
@@ -17,6 +17,14 @@ def build_scene(
|
|||||||
) -> Scene:
|
) -> Scene:
|
||||||
ui_elements = ui_elements or []
|
ui_elements = ui_elements or []
|
||||||
ocr_elements = ocr_elements or []
|
ocr_elements = ocr_elements or []
|
||||||
|
|
||||||
|
scale = _detect_pixel_scale(ui_elements, screen_width)
|
||||||
|
report_width, report_height = screen_width, screen_height
|
||||||
|
if abs(scale - 1.0) > 1e-6:
|
||||||
|
ocr_elements = [_scale_element(element, 1.0 / scale) for element in ocr_elements]
|
||||||
|
report_width = round(screen_width / scale)
|
||||||
|
report_height = round(screen_height / scale)
|
||||||
|
|
||||||
merged: list[SceneElement] = []
|
merged: list[SceneElement] = []
|
||||||
used_ocr: set[int] = set()
|
used_ocr: set[int] = set()
|
||||||
|
|
||||||
@@ -48,8 +56,8 @@ def build_scene(
|
|||||||
merged.append(_with_id(ocr_element, f"ocr-{ocr_index:03d}"))
|
merged.append(_with_id(ocr_element, f"ocr-{ocr_index:03d}"))
|
||||||
|
|
||||||
return Scene(
|
return Scene(
|
||||||
width=screen_width,
|
width=report_width,
|
||||||
height=screen_height,
|
height=report_height,
|
||||||
elements=merged,
|
elements=merged,
|
||||||
ocr_elements=list(ocr_elements),
|
ocr_elements=list(ocr_elements),
|
||||||
)
|
)
|
||||||
@@ -82,6 +90,41 @@ def infer_png_size(image: bytes | str | Path | None) -> tuple[int, int]:
|
|||||||
return (0, 0)
|
return (0, 0)
|
||||||
|
|
||||||
|
|
||||||
|
def _detect_pixel_scale(ui_elements: list[SceneElement], screen_width: int) -> float:
|
||||||
|
"""Detect the ratio between screenshot pixels and the UI tree's own unit (e.g. iOS points).
|
||||||
|
|
||||||
|
XCUITest reports UI tree bounds in logical points, which are half (or a
|
||||||
|
third, on some devices) of the screenshot's pixel dimensions on Retina
|
||||||
|
displays. Android's UiAutomator2 bounds already match screenshot pixels
|
||||||
|
1:1, so this returns 1.0 (no-op) for it. The first element rooted at
|
||||||
|
(0, 0) is used as the reference frame rather than the largest element,
|
||||||
|
since some system overlay elements report bounds that extend past the
|
||||||
|
visible screen.
|
||||||
|
"""
|
||||||
|
if screen_width <= 0:
|
||||||
|
return 1.0
|
||||||
|
for element in ui_elements:
|
||||||
|
bounds = element.bounds
|
||||||
|
if bounds.x == 0 and bounds.y == 0 and bounds.width > 0:
|
||||||
|
scale = screen_width / bounds.width
|
||||||
|
if scale > 0:
|
||||||
|
return scale
|
||||||
|
return 1.0
|
||||||
|
|
||||||
|
|
||||||
|
def _scale_element(element: SceneElement, factor: float) -> SceneElement:
|
||||||
|
bounds = element.bounds
|
||||||
|
return replace(
|
||||||
|
element,
|
||||||
|
bounds=Bounds(
|
||||||
|
x=bounds.x * factor,
|
||||||
|
y=bounds.y * factor,
|
||||||
|
width=bounds.width * factor,
|
||||||
|
height=bounds.height * factor,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _with_id(element: SceneElement, fallback_id: str) -> SceneElement:
|
def _with_id(element: SceneElement, fallback_id: str) -> SceneElement:
|
||||||
if element.id:
|
if element.id:
|
||||||
return element
|
return element
|
||||||
|
|||||||
+9
-2
@@ -134,11 +134,18 @@ class TaskRunner:
|
|||||||
):
|
):
|
||||||
return self._complete_task(task)
|
return self._complete_task(task)
|
||||||
|
|
||||||
for step in steps:
|
for step_index, step in enumerate(steps):
|
||||||
if should_stop is not None and should_stop():
|
if should_stop is not None and should_stop():
|
||||||
return self._interrupt_task(task)
|
return self._interrupt_task(task)
|
||||||
executable_step = self._step_for_device(step, task.device_id)
|
executable_step = self._step_for_device(step, task.device_id)
|
||||||
before_screenshot = self._planning_screenshot(task.device_id)
|
if step_index == 0 and screenshot is not None:
|
||||||
|
# Reuse the screenshot already captured for planning instead of
|
||||||
|
# taking a new one, so the "before action" image shown alongside
|
||||||
|
# `scene`'s OCR/UI-tree overlay matches what was actually planned
|
||||||
|
# against (avoids drift from the LLM planning round-trip).
|
||||||
|
before_screenshot = screenshot
|
||||||
|
else:
|
||||||
|
before_screenshot = self._planning_screenshot(task.device_id)
|
||||||
result = self.executor.execute(
|
result = self.executor.execute(
|
||||||
executable_step,
|
executable_step,
|
||||||
context=context,
|
context=context,
|
||||||
|
|||||||
@@ -43,3 +43,95 @@ def test_scene_builder_merges_overlapping_ocr_into_ui_element() -> None:
|
|||||||
assert scene.elements[1].text == "Footer"
|
assert scene.elements[1].text == "Footer"
|
||||||
assert [element.text for element in scene.ocr_elements] == ["Search", "Footer"]
|
assert [element.text for element in scene.ocr_elements] == ["Search", "Footer"]
|
||||||
assert bbox_iou(ui_button.bounds, ocr_label.bounds) > 0.5
|
assert bbox_iou(ui_button.bounds, ocr_label.bounds) > 0.5
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_scene_rescales_ocr_bounds_from_points_to_pixels_mismatch() -> None:
|
||||||
|
# Real numbers observed on an iOS Retina (2x) device: XCUITest reports the
|
||||||
|
# UI tree in points (414x896) while the screenshot (and therefore OCR) is
|
||||||
|
# in pixels (828x1792).
|
||||||
|
root = SceneElement(
|
||||||
|
id="ui-000",
|
||||||
|
type="application",
|
||||||
|
text=None,
|
||||||
|
bounds=Bounds(0, 0, 414, 896),
|
||||||
|
confidence=1.0,
|
||||||
|
source="ui",
|
||||||
|
)
|
||||||
|
button = SceneElement(
|
||||||
|
id="ui-button",
|
||||||
|
type="button",
|
||||||
|
text=None,
|
||||||
|
bounds=Bounds(20, 40, 100, 40),
|
||||||
|
confidence=1.0,
|
||||||
|
source="ui",
|
||||||
|
)
|
||||||
|
ocr_label = SceneElement(
|
||||||
|
id="ocr-label",
|
||||||
|
type="text",
|
||||||
|
text="Search",
|
||||||
|
bounds=Bounds(40, 80, 200, 80), # pixel-space, matches `button` once halved
|
||||||
|
confidence=0.9,
|
||||||
|
source="ocr",
|
||||||
|
)
|
||||||
|
|
||||||
|
scene = build_scene(
|
||||||
|
screen_width=828,
|
||||||
|
screen_height=1792,
|
||||||
|
ui_elements=[root, button],
|
||||||
|
ocr_elements=[ocr_label],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert scene.width == 414
|
||||||
|
assert scene.height == 896
|
||||||
|
fused_button = next(e for e in scene.elements if e.id == "ui-button")
|
||||||
|
assert fused_button.text == "Search"
|
||||||
|
|
||||||
|
ocr_only = SceneElement(
|
||||||
|
id="ocr-only",
|
||||||
|
type="text",
|
||||||
|
text="Footer",
|
||||||
|
bounds=Bounds(200, 1600, 100, 40),
|
||||||
|
confidence=0.8,
|
||||||
|
source="ocr",
|
||||||
|
)
|
||||||
|
scene_with_extra = build_scene(
|
||||||
|
screen_width=828,
|
||||||
|
screen_height=1792,
|
||||||
|
ui_elements=[root, button],
|
||||||
|
ocr_elements=[ocr_label, ocr_only],
|
||||||
|
)
|
||||||
|
footer = next(
|
||||||
|
e for e in scene_with_extra.ocr_elements if e.text == "Footer"
|
||||||
|
)
|
||||||
|
assert footer.bounds == Bounds(100, 800, 50, 20)
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_scene_is_noop_when_ui_tree_already_matches_pixel_scale() -> None:
|
||||||
|
# Android's UiAutomator2 bounds already match screenshot pixels 1:1.
|
||||||
|
root = SceneElement(
|
||||||
|
id="ui-000",
|
||||||
|
type="application",
|
||||||
|
text=None,
|
||||||
|
bounds=Bounds(0, 0, 1080, 2280),
|
||||||
|
confidence=1.0,
|
||||||
|
source="ui",
|
||||||
|
)
|
||||||
|
ocr_label = SceneElement(
|
||||||
|
id="ocr-label",
|
||||||
|
type="text",
|
||||||
|
text="Search",
|
||||||
|
bounds=Bounds(40, 80, 200, 80),
|
||||||
|
confidence=0.9,
|
||||||
|
source="ocr",
|
||||||
|
)
|
||||||
|
|
||||||
|
scene = build_scene(
|
||||||
|
screen_width=1080,
|
||||||
|
screen_height=2280,
|
||||||
|
ui_elements=[root],
|
||||||
|
ocr_elements=[ocr_label],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert scene.width == 1080
|
||||||
|
assert scene.height == 2280
|
||||||
|
assert scene.ocr_elements[0].bounds == Bounds(40, 80, 200, 80)
|
||||||
|
|||||||
+54
-2
@@ -158,12 +158,64 @@ def test_task_runner_persists_action_evidence_and_raw_ocr(tmp_path) -> None:
|
|||||||
|
|
||||||
assert result.status == "completed"
|
assert result.status == "completed"
|
||||||
record = timeline.read(result.id)[0]
|
record = timeline.read(result.id)[0]
|
||||||
assert Path(record["before_screenshot_path"]).read_bytes() == b"before-action"
|
# The first step of a plan batch reuses the screenshot already captured
|
||||||
assert Path(record["after_screenshot_path"]).read_bytes() == b"after-action"
|
# for planning as its "before action" evidence, instead of taking a new
|
||||||
|
# one, so it can't drift from what the planner actually saw.
|
||||||
|
assert Path(record["before_screenshot_path"]).read_bytes() == b"planning"
|
||||||
|
assert Path(record["after_screenshot_path"]).read_bytes() == b"before-action"
|
||||||
assert record["tool_call"]["description"] == "tap search"
|
assert record["tool_call"]["description"] == "tap search"
|
||||||
assert record["ocr_results"][0]["text"] == "Search"
|
assert record["ocr_results"][0]["text"] == "Search"
|
||||||
|
|
||||||
|
|
||||||
|
def test_task_runner_only_reuses_planning_screenshot_for_first_step_in_batch(
|
||||||
|
tmp_path,
|
||||||
|
) -> None:
|
||||||
|
scene = Scene(width=10, height=20, elements=[])
|
||||||
|
|
||||||
|
class BatchPlanner(Planner):
|
||||||
|
def plan(self, *, goal, scene, context):
|
||||||
|
if context.step_results:
|
||||||
|
return []
|
||||||
|
return [
|
||||||
|
PlannedStep(action="tap", description="first", args={}),
|
||||||
|
PlannedStep(action="tap", description="second", args={}),
|
||||||
|
]
|
||||||
|
|
||||||
|
def goal_reached(self, *, goal, scene, context):
|
||||||
|
return len(context.step_results) >= 2
|
||||||
|
|
||||||
|
screenshots = iter(
|
||||||
|
[
|
||||||
|
b"planning", # vision screenshot + step 1's reused before_screenshot
|
||||||
|
b"after-step1",
|
||||||
|
b"before-step2", # step 2 has no matching planning-time capture
|
||||||
|
b"after-step2",
|
||||||
|
]
|
||||||
|
)
|
||||||
|
timeline = Timeline(ArtifactStore(tmp_path / "history"))
|
||||||
|
runner = TaskRunner(
|
||||||
|
planner=BatchPlanner(),
|
||||||
|
executor=Executor(
|
||||||
|
tools={"tap": lambda **kwargs: {"ok": True}},
|
||||||
|
config=ExecutorConfig(max_retries=1, backoff_seconds=0),
|
||||||
|
),
|
||||||
|
timeline=timeline,
|
||||||
|
config=TaskRunnerConfig(max_steps=2),
|
||||||
|
observer=lambda device_id: scene,
|
||||||
|
screenshot_provider=lambda device_id: next(screenshots),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = runner.run(Task(goal="do two things", device_id="iphone-1"))
|
||||||
|
|
||||||
|
assert result.status == "completed"
|
||||||
|
records = timeline.read(result.id)
|
||||||
|
# Step 1 reuses the screenshot already captured for planning.
|
||||||
|
assert Path(records[0]["before_screenshot_path"]).read_bytes() == b"planning"
|
||||||
|
# Step 2 has no matching "at planning time" screenshot (step 1 already
|
||||||
|
# ran), so it must take a fresh one rather than reuse stale bytes.
|
||||||
|
assert Path(records[1]["before_screenshot_path"]).read_bytes() == b"before-step2"
|
||||||
|
|
||||||
|
|
||||||
def test_task_runner_persists_ui_tree_elements_from_fused_scene(tmp_path) -> None:
|
def test_task_runner_persists_ui_tree_elements_from_fused_scene(tmp_path) -> None:
|
||||||
scene = Scene(
|
scene = Scene(
|
||||||
width=10,
|
width=10,
|
||||||
|
|||||||
Reference in New Issue
Block a user