fix(computer_use): keep capture responses inside the tool-result budget and surface coordinate-space + typed-page hints (#85037)
This commit is contained in:
parent
08606fc231
commit
6c9d6d9d5b
|
|
@ -2129,3 +2129,142 @@ class TestStartupTimeoutPhaseDetail:
|
|||
msg = str(e)
|
||||
assert "stuck in phase: mcp-initialize" in msg
|
||||
assert "computer-use doctor" in msg
|
||||
|
||||
|
||||
class TestCapturePayloadBudget:
|
||||
"""Element labels and the aux-vision branch must respect response budgets.
|
||||
|
||||
Regression tests for the Discord/Electron capture blowup: UIA exposes
|
||||
entire message bodies as element labels, so a single capture response
|
||||
exceeded 170KB and the model never saw the elements it needed.
|
||||
"""
|
||||
|
||||
def test_element_label_is_capped_in_json(self):
|
||||
from tools.computer_use.backend import UIElement
|
||||
from tools.computer_use.tool import _MAX_ELEMENT_LABEL_CHARS, _element_to_dict
|
||||
|
||||
e = UIElement(index=3, role="Document", label="m" * 5000,
|
||||
bounds=(0, 0, 100, 100), app="chrome.exe")
|
||||
d = _element_to_dict(e)
|
||||
assert len(d["label"]) == _MAX_ELEMENT_LABEL_CHARS
|
||||
assert d["label_truncated"] is True
|
||||
|
||||
def test_short_label_not_flagged(self):
|
||||
from tools.computer_use.backend import UIElement
|
||||
from tools.computer_use.tool import _element_to_dict
|
||||
|
||||
d = _element_to_dict(UIElement(index=0, role="Button", label="OK",
|
||||
bounds=(0, 0, 10, 10), app=""))
|
||||
assert d["label"] == "OK"
|
||||
assert "label_truncated" not in d
|
||||
|
||||
def test_aux_vision_branch_respects_element_cap(self):
|
||||
"""The aux-vision payload must carry the same capped element list as
|
||||
every other capture branch, not the full untruncated tree."""
|
||||
from tools.computer_use.backend import CaptureResult, UIElement
|
||||
from tools.computer_use import tool as cu_tool
|
||||
|
||||
elements = [
|
||||
UIElement(index=i, role="Button", label=f"btn{i}",
|
||||
bounds=(0, 0, 10, 10), app="")
|
||||
for i in range(50)
|
||||
]
|
||||
cap = CaptureResult(mode="som", width=1024, height=768,
|
||||
png_b64="iVBORw0KGgo=", elements=elements,
|
||||
app="X", window_title="t", png_bytes_len=10)
|
||||
with patch("model_tools._run_async",
|
||||
return_value=json.dumps({"analysis": "a screen"})):
|
||||
out = cu_tool._route_capture_through_aux_vision(
|
||||
cap, "summary",
|
||||
visible_elements=elements[:5], truncated_elements=45,
|
||||
)
|
||||
assert out is not None
|
||||
payload = json.loads(out)
|
||||
assert len(payload["elements"]) == 5
|
||||
assert payload["total_elements"] == 50
|
||||
assert payload["truncated_elements"] == 45
|
||||
|
||||
|
||||
class TestBoundsSpaceNote:
|
||||
def test_note_present_when_bounds_exceed_image(self):
|
||||
from tools.computer_use.backend import UIElement
|
||||
from tools.computer_use.tool import _bounds_space_note
|
||||
|
||||
# Live repro: 1455x791 screenshot, element bounds out to x=3840
|
||||
# (native 4K desktop space).
|
||||
elems = [UIElement(index=0, role="Button", label="Close",
|
||||
bounds=(3771, 0, 69, 60), app="")]
|
||||
note = _bounds_space_note(elems, 1455, 791)
|
||||
assert note is not None
|
||||
assert "native desktop coordinates" in note
|
||||
|
||||
def test_no_note_when_spaces_match(self):
|
||||
from tools.computer_use.backend import UIElement
|
||||
from tools.computer_use.tool import _bounds_space_note
|
||||
|
||||
elems = [UIElement(index=0, role="Button", label="OK",
|
||||
bounds=(10, 10, 50, 20), app="")]
|
||||
assert _bounds_space_note(elems, 1455, 791) is None
|
||||
|
||||
def test_no_note_for_empty_or_degenerate(self):
|
||||
from tools.computer_use.backend import UIElement
|
||||
from tools.computer_use.tool import _bounds_space_note
|
||||
|
||||
assert _bounds_space_note([], 1455, 791) is None
|
||||
zero = [UIElement(index=0, role="B", label="x",
|
||||
bounds=(0, 0, 0, 0), app="")]
|
||||
assert _bounds_space_note(zero, 1455, 791) is None
|
||||
assert _bounds_space_note(zero, 0, 0) is None
|
||||
|
||||
|
||||
class TestEscalationEnrichment:
|
||||
"""Browser-class background_unavailable refusals gain a typed-page hint."""
|
||||
|
||||
def _refusal(self, **overrides):
|
||||
from tools.computer_use.backend import ActionResult
|
||||
|
||||
kw = dict(
|
||||
ok=False, action="type_text", message="refused",
|
||||
code="background_unavailable",
|
||||
escalation={"recommended": "foreground", "reason": "dropped"},
|
||||
meta={"event_kind": "text_input",
|
||||
"target_class": "Chrome_WidgetWin_1"},
|
||||
)
|
||||
kw.update(overrides)
|
||||
return ActionResult(**kw)
|
||||
|
||||
def test_browser_text_refusal_gains_page_alternative(self):
|
||||
from tools.computer_use.tool import _enrich_escalation
|
||||
|
||||
enriched = _enrich_escalation(self._refusal())
|
||||
# Driver's recommendation is never overridden — only augmented.
|
||||
assert enriched["recommended"] == "foreground"
|
||||
assert enriched["alternative"] == "page"
|
||||
assert "cua_browser_type" in enriched["alternative_hint"]
|
||||
|
||||
def test_non_browser_target_untouched(self):
|
||||
from tools.computer_use.tool import _enrich_escalation
|
||||
|
||||
res = self._refusal(meta={"event_kind": "text_input",
|
||||
"target_class": "Notepad"})
|
||||
assert "alternative" not in _enrich_escalation(res)
|
||||
|
||||
def test_non_foreground_recommendation_untouched(self):
|
||||
from tools.computer_use.tool import _enrich_escalation
|
||||
|
||||
res = self._refusal(escalation={"recommended": "px"})
|
||||
assert "alternative" not in _enrich_escalation(res)
|
||||
|
||||
def test_missing_escalation_passthrough(self):
|
||||
from tools.computer_use.backend import ActionResult
|
||||
from tools.computer_use.tool import _enrich_escalation
|
||||
|
||||
assert _enrich_escalation(
|
||||
ActionResult(ok=True, action="click", message="ok")) is None
|
||||
|
||||
def test_enrichment_survives_action_payload(self):
|
||||
from tools.computer_use.tool import _action_payload
|
||||
|
||||
payload = _action_payload(self._refusal())
|
||||
assert payload["escalation"]["alternative"] == "page"
|
||||
assert payload["verdict"]["decision"] == "escalate"
|
||||
|
|
|
|||
|
|
@ -824,8 +824,9 @@ def _action_payload(res: ActionResult) -> Dict[str, Any]:
|
|||
payload["verified"] = res.verified
|
||||
if res.effect is not None:
|
||||
payload["effect"] = res.effect
|
||||
if res.escalation is not None:
|
||||
payload["escalation"] = res.escalation
|
||||
escalation = _enrich_escalation(res)
|
||||
if escalation is not None:
|
||||
payload["escalation"] = escalation
|
||||
if res.path is not None:
|
||||
payload["path"] = res.path
|
||||
if res.degraded is not None:
|
||||
|
|
@ -844,6 +845,53 @@ def _text_response(res: ActionResult) -> str:
|
|||
return json.dumps(_action_payload(res))
|
||||
|
||||
|
||||
# Window classes of browsers whose page content the typed cua_browser_* route
|
||||
# can drive with trusted input and ZERO focus steal. When background text
|
||||
# delivery is refused for one of these surfaces, the driver's only hint is
|
||||
# "foreground" (it doesn't know Hermes has a typed page route), so the model
|
||||
# flashes the user's window to front for every keystroke batch. The hint below
|
||||
# offers the no-flash rung first; foreground remains valid for browser chrome,
|
||||
# native dialogs, and anything the typed route can't bind exactly.
|
||||
_TYPED_BROWSER_WINDOW_CLASSES = {
|
||||
"chrome_widgetwin_1", # Chrome, Edge, Brave, Electron-embedded Chromium
|
||||
"mozillawindowclass", # Firefox
|
||||
}
|
||||
|
||||
|
||||
def _enrich_escalation(res: ActionResult) -> Optional[Dict[str, Any]]:
|
||||
"""Return the driver's escalation dict, adding a typed-page alternative.
|
||||
|
||||
Purely additive: never changes the driver's `recommended` rung, only
|
||||
appends `alternative`/`alternative_hint` when the refused target is a
|
||||
known browser window class and the refused event is page-directed input
|
||||
(typing/keys into page content). The model can then try the
|
||||
`cua_browser_*` route — trusted input, no window flash — before a
|
||||
foreground escalation, per the documented ladder ordering.
|
||||
"""
|
||||
escalation = res.escalation
|
||||
if not isinstance(escalation, dict):
|
||||
return escalation
|
||||
if escalation.get("recommended") != "foreground":
|
||||
return escalation
|
||||
meta = res.meta or {}
|
||||
target_class = str(meta.get("target_class") or "").lower()
|
||||
if target_class not in _TYPED_BROWSER_WINDOW_CLASSES:
|
||||
return escalation
|
||||
if meta.get("event_kind") not in {"text_input", "key_press"}:
|
||||
return escalation
|
||||
enriched = dict(escalation)
|
||||
enriched["alternative"] = "page"
|
||||
enriched["alternative_hint"] = (
|
||||
"target is a browser window: if the input goes into PAGE content "
|
||||
"(not browser chrome or a native dialog), the typed cua_browser_* "
|
||||
"route can deliver it without any window flash — bind with "
|
||||
"cua_browser_state (exact pid/window_id), then cua_browser_type. "
|
||||
"Use foreground only for chrome/native surfaces or if typed binding "
|
||||
"is unavailable."
|
||||
)
|
||||
return enriched
|
||||
|
||||
|
||||
# Default cap for the AX `elements` array returned by capture. Dense UIs
|
||||
# (Electron apps, Obsidian, JetBrains IDEs) can publish 500+ AX nodes, which
|
||||
# can exhaust session context after a single capture. The model-facing
|
||||
|
|
@ -939,6 +987,7 @@ def _capture_response(cap: CaptureResult, max_elements: int = _DEFAULT_MAX_ELEME
|
|||
image_dimensions = _image_dimensions_from_b64(cap.png_b64 or "") if cap.png_b64 else None
|
||||
response_width = image_dimensions[0] if image_dimensions else cap.width
|
||||
response_height = image_dimensions[1] if image_dimensions else cap.height
|
||||
bounds_note = _bounds_space_note(visible_elements, response_width, response_height)
|
||||
image_too_small = bool(
|
||||
image_dimensions
|
||||
and (
|
||||
|
|
@ -958,6 +1007,8 @@ def _capture_response(cap: CaptureResult, max_elements: int = _DEFAULT_MAX_ELEME
|
|||
+ (f" window={cap.window_title!r}" if cap.window_title else ""),
|
||||
f"{total_elements} interactable element(s):",
|
||||
]
|
||||
if bounds_note:
|
||||
summary_lines.append(f" ({bounds_note})")
|
||||
if element_index:
|
||||
summary_lines.extend(element_index)
|
||||
# Multimodal and AX paths both reference `summary`; build it once up-front
|
||||
|
|
@ -981,7 +1032,11 @@ def _capture_response(cap: CaptureResult, max_elements: int = _DEFAULT_MAX_ELEME
|
|||
# main models tripped HTTP 404 / 400 at the provider boundary even
|
||||
# when auxiliary.vision was explicitly configured to handle this.
|
||||
if _should_route_through_aux_vision():
|
||||
routed = _route_capture_through_aux_vision(cap, summary)
|
||||
routed = _route_capture_through_aux_vision(
|
||||
cap, summary,
|
||||
visible_elements=visible_elements,
|
||||
truncated_elements=truncated_elements,
|
||||
)
|
||||
if routed is not None:
|
||||
return routed
|
||||
# Aux routing was requested but failed (vision node down, aux call
|
||||
|
|
@ -1170,6 +1225,9 @@ def _capture_after_mode() -> str:
|
|||
def _route_capture_through_aux_vision(
|
||||
cap: CaptureResult,
|
||||
summary: str,
|
||||
*,
|
||||
visible_elements: Optional[List[UIElement]] = None,
|
||||
truncated_elements: int = 0,
|
||||
) -> Optional[str]:
|
||||
"""Pre-analyse the captured PNG via ``vision_analyze`` and return a text result.
|
||||
|
||||
|
|
@ -1261,17 +1319,27 @@ def _route_capture_through_aux_vision(
|
|||
if not analysis_text:
|
||||
return None
|
||||
|
||||
return json.dumps({
|
||||
# Respect the same element cap as every other capture branch. Before this,
|
||||
# the aux-vision path dumped cap.elements in full — silently bypassing
|
||||
# max_elements exactly when a non-vision main model was configured, so a
|
||||
# dense Electron UI (Discord, Slack, IDEs) could blow the response budget
|
||||
# on this branch alone.
|
||||
elements_out = cap.elements if visible_elements is None else visible_elements
|
||||
payload: Dict[str, Any] = {
|
||||
"mode": cap.mode,
|
||||
"width": cap.width,
|
||||
"height": cap.height,
|
||||
"app": cap.app,
|
||||
"window_title": cap.window_title,
|
||||
"elements": [_element_to_dict(e) for e in cap.elements],
|
||||
"elements": [_element_to_dict(e) for e in elements_out],
|
||||
"total_elements": len(cap.elements),
|
||||
"summary": summary,
|
||||
"vision_analysis": analysis_text,
|
||||
"vision_analysis_routed_via": "auxiliary.vision",
|
||||
})
|
||||
}
|
||||
if truncated_elements:
|
||||
payload["truncated_elements"] = truncated_elements
|
||||
return json.dumps(payload)
|
||||
|
||||
|
||||
def _maybe_follow_capture(
|
||||
|
|
@ -1330,14 +1398,73 @@ def _format_elements(elements: List[UIElement], max_lines: int = 40) -> List[str
|
|||
return out
|
||||
|
||||
|
||||
# Element labels come straight from the platform accessibility tree, which on
|
||||
# some apps (Discord/Slack via UIA, Electron chat clients generally) exposes
|
||||
# ENTIRE message bodies / document text as the accessible name of a node.
|
||||
# 100 elements x multi-KB labels made single capture responses exceed 170KB —
|
||||
# blowing the tool-result budget so the model never saw the elements it needed,
|
||||
# and leaking full private chat text into context. The summary line has always
|
||||
# truncated to 60 chars; this applies a (more generous) cap to the JSON
|
||||
# `elements` array too. Labels are for identifying a control, not for reading
|
||||
# page content — captures are not a text-extraction surface.
|
||||
_MAX_ELEMENT_LABEL_CHARS = 120
|
||||
|
||||
|
||||
def _bounds_space_note(
|
||||
elements: List[UIElement], image_width: int, image_height: int,
|
||||
) -> Optional[str]:
|
||||
"""Warn when element bounds live in a different coordinate space.
|
||||
|
||||
On HiDPI/scaled displays (common on Windows + macOS retina), cua-driver
|
||||
reports AX element bounds in native desktop coordinates while the
|
||||
screenshot is captured/downscaled to a smaller pixel grid. Nothing in the
|
||||
response related the two, so models reading a position off the screenshot
|
||||
and clicking by coordinate= missed by the scale factor (e.g. 2.6x on a
|
||||
4K display with a 1455px-wide screenshot). Element bounds are what
|
||||
click(coordinate=...) expects; the note makes that explicit whenever the
|
||||
two spaces visibly diverge.
|
||||
"""
|
||||
if not elements or image_width <= 0 or image_height <= 0:
|
||||
return None
|
||||
max_x = 0
|
||||
max_y = 0
|
||||
for e in elements:
|
||||
try:
|
||||
x, y, w, h = e.bounds
|
||||
except (TypeError, ValueError):
|
||||
continue
|
||||
max_x = max(max_x, int(x) + int(w))
|
||||
max_y = max(max_y, int(y) + int(h))
|
||||
if max_x <= 0 and max_y <= 0:
|
||||
return None
|
||||
# 5% slack: window chrome can hang a few px past the captured frame
|
||||
# without implying a different coordinate space.
|
||||
if max_x <= image_width * 1.05 and max_y <= image_height * 1.05:
|
||||
return None
|
||||
return (
|
||||
f"element bounds are in native desktop coordinates (extend to "
|
||||
f"~{max_x}x{max_y}), NOT screenshot pixels ({image_width}x"
|
||||
f"{image_height}). coordinate= clicks expect the native space — "
|
||||
"derive click points from element bounds, or scale screenshot "
|
||||
"positions up accordingly"
|
||||
)
|
||||
|
||||
|
||||
def _element_to_dict(e: UIElement) -> Dict[str, Any]:
|
||||
return {
|
||||
label = e.label
|
||||
truncated = len(label) > _MAX_ELEMENT_LABEL_CHARS
|
||||
if truncated:
|
||||
label = label[:_MAX_ELEMENT_LABEL_CHARS]
|
||||
out: Dict[str, Any] = {
|
||||
"index": e.index,
|
||||
"role": e.role,
|
||||
"label": e.label,
|
||||
"label": label,
|
||||
"bounds": list(e.bounds),
|
||||
"app": e.app,
|
||||
}
|
||||
if truncated:
|
||||
out["label_truncated"] = True
|
||||
return out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in New Issue