From 7f439f0db5ccf0641efc28d9a72bd838c7a6090a Mon Sep 17 00:00:00 2001 From: Jerry Yan <792602257@qq.com> Date: Wed, 15 Jul 2026 16:12:53 +0800 Subject: [PATCH] fix(perception): reconcile points/pixels scale and stale overlay screenshot 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. --- perception/scene_builder.py | 47 ++++++++++++++++++- runtime/task.py | 11 ++++- tests/test_scene_builder.py | 92 +++++++++++++++++++++++++++++++++++++ tests/test_task_loop.py | 56 +++++++++++++++++++++- 4 files changed, 200 insertions(+), 6 deletions(-) diff --git a/perception/scene_builder.py b/perception/scene_builder.py index a9a5542..784c4d5 100644 --- a/perception/scene_builder.py +++ b/perception/scene_builder.py @@ -17,6 +17,14 @@ def build_scene( ) -> Scene: ui_elements = ui_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] = [] used_ocr: set[int] = set() @@ -48,8 +56,8 @@ def build_scene( merged.append(_with_id(ocr_element, f"ocr-{ocr_index:03d}")) return Scene( - width=screen_width, - height=screen_height, + width=report_width, + height=report_height, elements=merged, ocr_elements=list(ocr_elements), ) @@ -82,6 +90,41 @@ def infer_png_size(image: bytes | str | Path | None) -> tuple[int, int]: 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: if element.id: return element diff --git a/runtime/task.py b/runtime/task.py index cc7fcd9..5a3a867 100644 --- a/runtime/task.py +++ b/runtime/task.py @@ -134,11 +134,18 @@ class TaskRunner: ): 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(): return self._interrupt_task(task) 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( executable_step, context=context, diff --git a/tests/test_scene_builder.py b/tests/test_scene_builder.py index e7d4566..3771b5b 100644 --- a/tests/test_scene_builder.py +++ b/tests/test_scene_builder.py @@ -43,3 +43,95 @@ def test_scene_builder_merges_overlapping_ocr_into_ui_element() -> None: assert scene.elements[1].text == "Footer" assert [element.text for element in scene.ocr_elements] == ["Search", "Footer"] 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) diff --git a/tests/test_task_loop.py b/tests/test_task_loop.py index 83dc63b..729dca0 100644 --- a/tests/test_task_loop.py +++ b/tests/test_task_loop.py @@ -158,12 +158,64 @@ def test_task_runner_persists_action_evidence_and_raw_ocr(tmp_path) -> None: assert result.status == "completed" record = timeline.read(result.id)[0] - assert Path(record["before_screenshot_path"]).read_bytes() == b"before-action" - assert Path(record["after_screenshot_path"]).read_bytes() == b"after-action" + # The first step of a plan batch reuses the screenshot already captured + # 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["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: scene = Scene( width=10,