fix(cua-driver): complete coordinate normalization for zoom/scroll/mouse tools#6610
Merged
Conversation
When CUA_DRIVER_RS_COORDINATE_SPACE=1 (0-1000 normalized coordinates), from_zoom=true clicks were skipping denormalization entirely, causing the model's 0-1000 values to be treated as raw pixel offsets in the zoom image — landing clicks at the wrong position. - Add ZOOM_SIZE_CACHE: cache zoom image dimensions (width/height) from zoom tool results, keyed by pid (matching platform ZoomRegistry) - denormalize_args: when from_zoom=true and cache exists, denormalize x/y against zoom image dimensions instead of skipping - rewrite_coord_desc: rewrite from_zoom description from "pixel coordinates" to "0-1000 normalized coordinates" so the model sends normalized values - ingest_zoom_size: new output hook called after zoom tool execution to populate the cache
…e tools Two additional fixes for the 0-1000 normalized coordinate mode: 1. denormalize_args now returns Result<(), String> and errors when a required size basis is missing (SIZE_CACHE, SCREEN_SIZE, DESKTOP_SCREENSHOT_SIZE, ZOOM_SIZE_CACHE), instead of silently passing through unconverted normalized coordinates that would land clicks at wrong positions. The error messages guide the model to call the appropriate query tool (get_window_state, get_screen_size, get_desktop_state, zoom) first. 2. Add scroll, mouse_button_down, mouse_button_up, mouse_drag to input_coord_fields so their x/y coordinates are denormalized in normalized mode. scroll x/y specify where to deliver the wheel event (not scroll amount); Linux mouse tools are stateful press/drag/release with window-local coordinates.
qwen-code-ci-bot
previously requested changes
Jul 9, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Hi @LaZzyMan — thanks for the PR!
The code changes look like a solid bug fix, but the PR body doesn't follow the pull request template. Several required sections are missing or renamed:
## Summary→ should be## What this PR does## Why it's needed— missing entirely## Test plan→ should be## Reviewer Test Planwith the sub-sections (How to verify,Evidence (Before & After),Tested on)## Risk & Scope— missing## Linked Issues— missing- Chinese translation (
<details>中文说明</details>) — missing
Could you update the PR body to match the template? It helps reviewers understand the motivation, verify the fix, and assess risk. Thanks!
中文说明
你好 @LaZzyMan，感谢提交！
代码改动看起来是一个扎实的 bug 修复，但 PR 正文没有按照 PR 模板 来写。有几个必需的部分缺失或被重命名了：
## Summary→ 应该是## What this PR does## Why it's needed— 完全缺失## Test plan→ 应该是## Reviewer Test Plan，包含子章节（How to verify、Evidence (Before & After)、Tested on）## Risk & Scope— 缺失## Linked Issues— 缺失- 中文翻译（
<details>中文说明</details>）— 缺失
请把 PR 正文改成模板格式，这样方便 reviewer 理解动机、验证修复、评估风险。谢谢！
— Qwen Code · qwen3.7-max
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
pomelo-nwu
requested changes
Jul 9, 2026
| if tool == "parallel_mouse_drag" { | ||
| if let Some(drags) = args.get_mut("drags").and_then(|v| v.as_array_mut()) { | ||
| let scale = coordinate_scale(); | ||
| let (dw, dh) = if screenshot_w > 0 { |
Collaborator
There was a problem hiding this comment.
[Critical] parallel_mouse_drag is broken in normalized mode for two reasons:
-
Always errors:
tool.rsextractspid/window_idfrom top-level args, but forparallel_mouse_dragthey live insidedrags[]items. Sopid=0,window_id=0,get_size(0, 0)returns(0, 0), andscreenshot_w=0triggers this error unconditionally (unless desktop_screenshot_size happens to be cached). -
Single basis for all items: Even when it doesn't error, all drag items share the same
(dw, dh)regardless of each item's ownwindow_id. Cross-window drags get wrong pixel coordinates.
The fix should resolve per-item size inside the loop:
if tool == "parallel_mouse_drag" {
if let Some(drags) = args.get_mut("drags").and_then(|v| v.as_array_mut()) {
let scale = coordinate_scale();
let pid = args.get("pid").and_then(|v| v.as_i64()).unwrap_or(0);
for item in drags.iter_mut() {
let item_wid = item.get("window_id")
.and_then(|v| v.as_u64()).unwrap_or(0);
let (dw, dh) = get_size(pid, item_wid).unwrap_or((0, 0));
if dw == 0 {
return Err(
"parallel_mouse_drag: coordinate normalization requires \
window screenshot size for each drag item. \
Call get_window_state first."
.to_string(),
);
}
// ... rest of per-item conversion
}
}
}— qwen3.7-max via Qwen Code /review
| ); | ||
| } | ||
| } | ||
| return Ok(()); |
Collaborator
There was a problem hiding this comment.
[Critical] When from_zoom=true is combined with parallel_mouse_drag, this early return silently skips all nested coordinate conversion:
input_coord_fields("parallel_mouse_drag")returns&[]- The conversion loop iterates zero times
has_coordsevaluates tofalse(no fields to check)return Ok(())fires before reaching theparallel_mouse_draghandler below
Result: nested drags[] coordinates (from_x, to_x, path, etc.) pass through unconverted with a success status — silent data corruption.
Fix: either explicitly reject this combination:
if tool == "parallel_mouse_drag" {
return Err("from_zoom is not supported for parallel_mouse_drag".to_string());
}Or move the parallel_mouse_drag nested handler before this early return.
— qwen3.7-max via Qwen Code /review
| // The from_x/to_x/path fields are inside drags[] so they can't be | ||
| // listed here; their descriptions are handled by the nested rewrite | ||
| // in rewrite_coord_desc below. | ||
| "parallel_mouse_drag" => &[], |
Collaborator
There was a problem hiding this comment.
[Critical] This comment is factually wrong on two counts:
- The entry IS
&[](empty), not "non-empty" - The "nested rewrite in rewrite_coord_desc below" does not exist —
rewrite_coord_desconly handles top-levelinputSchema.properties, not nesteddrags[].items.properties
This means parallel_mouse_drag's nested field descriptions (from_x, to_x, path, x_from, x_to inside drags[] items) keep their original "window-local pixels" wording even in normalized mode. The model receives contradictory signals — top-level description says "normalized" but nested fields say "pixels" — which can cause the model to send raw pixel values that get double-converted.
Fix the comment and add nested schema rewriting, or explicitly document this gap.
— qwen3.7-max via Qwen Code /review
| }; | ||
| for item in drags.iter_mut() { | ||
| // from_x/from_y/to_x/to_y | ||
| for (field, is_x) in &[("from_x", true), ("from_y", false), ("to_x", true), ("to_y", false)] { |
Collaborator
There was a problem hiding this comment.
[Critical] The fn-based drag path uses x_from and x_to fields (domain bounds described in the schema as "window-local pixels"), but the conversion loop above only handles from_x, from_y, to_x, to_y, and path.
When a model uses the fn expression path in normalized mode, x_from/x_to are sent as 0–1000 normalized values but passed to sample_function without conversion — the function evaluates over the wrong domain.
Add conversion for x_from/x_to:
for (field, is_x) in &[("from_x", true), ("from_y", false),
("to_x", true), ("to_y", false),
("x_from", true), ("x_to", true)] {— qwen3.7-max via Qwen Code /review
Collaborator
Suggestions — commit
|
… mode normalize_result was rewriting get_window_state and get_desktop_state screenshot_width/height to 1000, conflicting with elements[].frame which stays in pixels. The model received contradictory coordinate information: screenshot dimensions said 1000 but element positions were in real pixels (e.g. x=1444). Query tool results should be returned unmodified. The model is guided to use 0-1000 coordinates through tool schema descriptions and MCP instructions, not by tampering with return values.
1. Stop rewriting screenshot_width/height to 1000 in normalized mode. Query results now return real pixel values; the model is guided to use 0-1000 coords through schema descriptions and instructions only. 2. Fix parallel_mouse_drag per-item window_id lookup: each drag item resolves its own (pid, window_id) from SIZE_CACHE instead of using the caller-level screenshot_w/h (which is always 0 since the tool has no top-level pid/window_id). 3. Add x_from/x_to conversion for fn-expression domain bounds. 4. Prevent from_zoom early return from skipping nested coord handler. 5. Fix misleading comments about "non-empty entry" and "nested rewrite".
pomelo-nwu
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Fixes multiple gaps in cua-driver's 0–1000 normalized coordinate mode (
CUA_DRIVER_RS_COORDINATE_SPACE=1):normalize_resultno longer overwritesscreenshot_width/heightto 1000 inget_window_state/get_desktop_stateresponses — this conflicted withelements[].framewhich stays in real pixels, giving the model contradictory coordinate info.ZOOM_SIZE_CACHE);from_zoom=trueclicks now denormalize against the cached zoom dimensions instead of being silently skipped. Rewritefrom_zoomschema description from "pixel coordinates" to "0–1000 normalized".denormalize_argsreturnsResultand errors when the required size basis is missing, instead of silently passing through unconverted 0–1000 values that land clicks at wrong positions.input_coord_fieldsfor coordinate conversion and description rewriting.(pid, window_id)SIZE_CACHE lookup instead of top-level args; convertx_from/x_to(fn domain bounds); preventfrom_zoomearly return from skipping nested handler.Why it's needed
Users enabling
CUA_DRIVER_RS_COORDINATE_SPACE=1for 0–1000 normalized coordinate mode reported that the mode was not fully effective: tool schema descriptions still said "pixel", zoom+from_zoom clicks landed at wrong positions, and some tools (scroll, Linux mouse tools) had no coordinate conversion at all. Thescreenshot_width/height → 1000rewrite also contradicted the real pixel values inelements[].frame, confusing the model.Reviewer Test Plan
How to verify
CUA_DRIVER_RS_COORDINATE_SPACE=1in cua-driver MCP env configget_window_state→ verifyscreenshot_width/heightreturns real pixel values (not 1000)zoomon a region → verify the returnedwidth/heightare real pixelsclick(from_zoom=true, x=500, y=500)→ should denormalize 500 against zoom image size and click the center of the zoom regionscroll(x=500, y=500, direction=down)→ should denormalize and scroll at the center of the windowtools/list→ all coordinate field descriptions should say "0–1000 normalized", includingfrom_zoomEvidence (Before & After)
Before: model sent pixel coords (180,180) for from_zoom click because
from_zoomdescription said "pixel coordinates";screenshot_widthwas rewritten to 1000 butelements[].framestayed in pixels.After: model sends normalized coords (500,498) for from_zoom click;
screenshot_widthreturns real value; all descriptions consistent.Verified via qwen-code headless E2E: sessions
0267ce02(before) →6e29f869(after, from_zoom click correct) →a36fc2f4(scroll with normalized coords).Tested on
Risk & Scope
normalize_resultno longer rewritesscreenshot_width/heightto 1000. Models that relied on seeingscreenshot_width=1000to decide coordinate scale will now see real pixel values — but the schema descriptions and MCP instructions already tell them to use 0–1000, so this should be a net improvement.cua-driver-core(coord_norm.rs, tool.rs, protocol.rs) + version bump in workspace Cargo.toml. No platform-layer changes.Linked Issues
User-reported: relative coordinate mode not fully effective (zoom clicks wrong, tool descriptions inconsistent, query results tampered).
中文说明
修复 cua-driver 0–1000 归一化坐标模式的多个缺陷：停止篡改查询结果的 screenshot_width/height；修复 zoom+from_zoom 点击坐标转换；缓存未命中时报错引导模型调用查询工具；补全 scroll/type_text/press_key/hotkey/Linux mouse 工具的坐标转换；修复 parallel_mouse_drag 的逐项窗口查找和 fn 域边界转换；全面修正工具描述中的像素措辞。