fix(core): reject fractional computer-use integer strings - #5500
Conversation
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Hi @tt-a1i, thanks for the fix — the code change itself looks clean and well-scoped (4-line fix + regression tests, exactly the right shape for this bug).
However, the PR body doesn't follow the pull request template. Several required sections are missing:
- What this PR does / Why it's needed — the
Summarysection covers this informally, but the template asks for these as separate headings - Reviewer Test Plan — with
How to verify,Evidence (Before & After), andTested on(OS table). This is the most important missing piece — reviewers need to know how to confirm the fix - Risk & Scope — even for a small fix, noting the risk/tradeoff helps reviewers
- Linked Issues —
Fixes #5499appears in the Summary but should be under theLinked Issuesheading - 中文说明 — the
<details>block with a Chinese translation
Could you update the PR description to match the template? The code changes look good — this is just about making it easier for maintainers to review and merge. 🙏
|
updated the PR description to match the template. thanks for the pointer. |
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, test plan included. On direction: this is a straightforward correctness fix. On approach: the diff is minimal — 2 files, +22/-1. One regex split by field type, three new test cases. No scope creep, no drive-by refactors. This is exactly the size this fix should be. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必填段落齐全，中英双语，测试计划完备。 方向：这是一个直接的正确性修复。 方案：diff 极简 — 2 个文件，+22/-1。按字段类型拆分一个正则，新增 3 个测试用例。没有范围蔓延，没有顺手重构。这正是这个 fix 应有的体量。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: I would split the regex validation by field type — use The PR does exactly this. The fix is a 4-line change in const matchesType =
fieldType === 'integer'
? /^-?\d+$/.test(trimmed)
: /^-?\d+(\.\d+)?$/.test(trimmed);
if (matchesType) { ... }No correctness issues, no regressions, no convention violations. The three new tests cover the key cases: integer string coercion still works, fractional strings are rejected for integer fields, and fractional strings still work for number fields. TestingRan Bug reproduction (main code + PR tests)After fix (PR code + PR tests)Bug confirmed, fix verified. All 40 tests pass. 中文说明代码审查独立方案：按字段类型拆分正则 — integer 用 PR 正是这样做的。 测试Bug 已确认（main 代码 + PR 测试：1 个测试失败， — Qwen Code · qwen3.7-max |
|
This is a clean, minimal bug fix that does exactly what it says. The independent proposal matches the PR's approach 1:1 — split the regex by field type, no other changes needed. The before/after test run confirms the bug and the fix. No concerns, no reservations. Ships it. ✅ 中文说明干净、极简的 bug 修复，完全如描述。独立方案与 PR 方案 1:1 吻合 — 按字段类型拆分正则，无需其他改动。前后测试对比确认了 bug 和修复。无顾虑，无保留。 可以合并 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local runtime verification — PR #5500 (
|
| schema type | input | main (before) | PR (after) |
|---|---|---|---|
integer |
"11.5" |
11 (number) 🐛 |
"11.5" (string) ✅ |
integer |
"-3.5" |
-3 (number) 🐛 |
"-3.5" (string) ✅ |
integer |
"11.0" |
11 (number) 🐛 |
"11.0" (string) ✅ |
integer |
"11" |
11 |
11 (same) |
integer |
" 12 " |
12 |
12 (same — trim still works) |
integer |
"abc" / "11abc" |
string (untouched) | string (same) |
number |
"11.5" |
11.5 (number) |
11.5 (same — control) |
number |
"-3.5" |
-3.5 |
-3.5 (same — control) |
string |
11 |
"11" |
"11" (same — control) |
3 integer+fractional inputs flip from a truncated number to a preserved string; every number/string/clean-integer case is identical → no collateral change.
A/B differential — real ComputerUseTool.validateToolParams() verdict
This is the user-facing effect the issue describes — does the malformed value get caught or silently mis-targeted?
| tool params | main (before) | PR (after) |
|---|---|---|
{ element_index: "11.5" } |
✅ PASS — accepted as 11 🐛 |
❌ REJECT — params/element_index must be integer |
{ element_index: "-3.5" } |
✅ PASS — accepted as -3 🐛 |
❌ REJECT |
{ element_index: "11" } |
PASS | PASS (valid integer) |
{ element_index: "abc" } |
REJECT | REJECT (garbage — same) |
{ x: "11.5" } |
PASS | PASS (number field — same) |
On main, element_index: "11.5" silently becomes 11 and passes validation — exactly the "targets a different UI element instead of failing" failure mode from #5499. On the PR it is rejected before reaching the tool.
Committed tests + teeth (counterfactual)
| check | result |
|---|---|
committed coerceTypes suite on PR (vitest) |
✅ 9 passed (incl. the 3 new tests) |
counterfactual — new test does not truncate fractional strings for integer fields vs main tool.ts |
❌ FAILS — expected 11 to be '11.5' |
The counterfactual failing proves the new test genuinely pins the fixed behavior.
Why the fix is sound
The only change is selecting the numeric regex by declared type: integer → /^-?\d+$/ (no decimal point), number → /^-?\d+(\.\d+)?$/ (decimals allowed). A fractional string no longer matches the integer regex, so it is left untouched and fails schema validation downstream — matching the schema contract. Clean integer strings, decimal number strings, whitespace trimming, and garbage handling are all unchanged.
🇨🇳 中文版（点击展开）
✅ 本地运行时验证 — PR #5500（fix(core): reject fractional computer-use integer strings）
结论：通过。 该修复解决了 #5499：integer schema 字段传入 "11.5" 这类小数字符串，不再被 parseInt 静默截断成 11，而是保持字符串，从而被 schema 校验拒绝；同时 number 字段仍然接受小数字符串。验证方式：驱动真实的 coerceTypes() 与真实的 ComputerUseTool.validateToolParams() 流水线（真实 ajv schema 校验），在 tmux 中对 PR 与 origin/main 做 A/B 对比。
测试方法
- 验证测试覆盖：(1) 导出的
coerceTypes()在(schemaType, value)矩阵上的行为；(2) 一个真实的ComputerUseTool实例调用validateToolParams()——也就是模型工具调用实际会走的coerce → SchemaValidator.validate路径。 - A/B = 同一个测试，分别在 PR 的
tool.ts（按类型选择正则）上跑一次，再把该文件精确还原回origin/main（共用小数正则 +parseInt）跑一次。两组之间只有这一处修复的差别。
A/B 差异对比 — coerceTypes(value) 输出
| schema 类型 | 输入 | main（修复前） | PR（修复后） |
|---|---|---|---|
integer |
"11.5" |
11 (number) 🐛 |
"11.5" (string) ✅ |
integer |
"-3.5" |
-3 (number) 🐛 |
"-3.5" (string) ✅ |
integer |
"11.0" |
11 (number) 🐛 |
"11.0" (string) ✅ |
integer |
"11" |
11 |
11 (相同) |
integer |
" 12 " |
12 |
12 (相同——trim 仍生效) |
integer |
"abc" / "11abc" |
字符串（不动） | 字符串（相同） |
number |
"11.5" |
11.5 (number) |
11.5 (相同——对照) |
number |
"-3.5" |
-3.5 |
-3.5 (相同——对照) |
string |
11 |
"11" |
"11" (相同——对照) |
3 个「整数字段 + 小数」输入从被截断的数字变成保留的字符串；所有 number/string/合法整数的情况完全一致 → 没有附带影响。
A/B 差异对比 — 真实 ComputerUseTool.validateToolParams() 判定
这正是 issue 描述的用户可见效果——畸形值会被拦截，还是被静默地误用？
| 工具参数 | main（修复前） | PR（修复后） |
|---|---|---|
{ element_index: "11.5" } |
✅ 通过——被当成 11 接受 🐛 |
❌ 拒绝——params/element_index must be integer |
{ element_index: "-3.5" } |
✅ 通过——被当成 -3 接受 🐛 |
❌ 拒绝 |
{ element_index: "11" } |
通过 | 通过 (合法整数) |
{ element_index: "abc" } |
拒绝 | 拒绝 (垃圾——相同) |
{ x: "11.5" } |
通过 | 通过 (number 字段——相同) |
在 main 上，element_index: "11.5" 会静默变成 11 并通过校验——正是 #5499 里「误点到另一个 UI 元素而不是校验失败」的故障模式。在 PR 上它会在到达工具之前被拒绝。
已提交测试 + 有效性（反事实）
| 检查项 | 结果 |
|---|---|
已提交 coerceTypes 套件在 PR 上（vitest） |
✅ 9 通过（含 3 个新增测试） |
反事实 —— 新测试 does not truncate fractional strings for integer fields 对 main 的 tool.ts |
❌ 失败 —— expected 11 to be '11.5' |
反事实失败证明新增测试真正锁定了修复后的行为。
修复为什么是正确的
唯一的改动是按声明类型选择数字正则：integer → /^-?\d+$/（不含小数点），number → /^-?\d+(\.\d+)?$/（允许小数）。小数字符串不再匹配整数正则，于是保持原值并在后续 schema 校验中失败——符合 schema 合约。合法整数字符串、小数 number 字符串、空白 trim、垃圾处理都保持不变。
Reproducible: real coerceTypes() + real ComputerUseTool.validateToolParams() (genuine ajv) driven by a vitest verify test, A/B origin/main vs PR 8a4a31c, plus the committed suite and a reverted-prod counterfactual — all in tmux. Working tree restored clean.
3 participants
What this PR does
This PR tightens computer-use argument coercion for integer fields so numeric strings like
"1.5"are rejected instead of being truncated to1. Decimal coercion remains allowed for schema fields that are actuallynumber.Why it's needed
Computer-use integer parameters are used for pixel coordinates and other count-like values. Accepting a fractional string and silently truncating it hides malformed model output, which can make tool calls behave differently from the schema contract.
Reviewer Test Plan
How to verify
Run the focused computer-use tool tests and confirm integer schema fields reject fractional numeric strings while number schema fields still accept decimal numeric strings.
Evidence (Before & After)
Before: an integer field value such as
"1.5"passed validation and was coerced withparseInt, becoming1.After: integer fields reject fractional numeric strings; existing number fields still coerce values like
"1.5"to1.5.Tested on
Environment (optional)
Local validation used the package test runner against
packages/core/src/tools/computer-use/tool.test.ts.Risk & Scope
Linked Issues
Fixes #5499
Testing
AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.
中文说明
这个 PR 做了什么
这个 PR 收紧了 computer-use 参数里整数类型字段的强制转换逻辑，让
"1.5"这类数字字符串被拒绝，而不是被截断成1。真正声明为number的字段仍然允许小数转换。为什么需要
computer-use 的整数参数会用于像素坐标和其他计数类值。把小数字符串静默截断会掩盖不符合 schema 的模型输出，也会让工具调用行为偏离 schema 合约。
Reviewer 测试计划
如何验证
运行聚焦的 computer-use tool 测试，确认 integer schema 字段会拒绝小数字符串，同时 number schema 字段仍然接受小数字符串。
前后证据
之前：integer 字段传入
"1.5"会通过验证，并被parseInt转成1。之后：integer 字段会拒绝小数字符串；已有 number 字段仍会把
"1.5"转成1.5。测试平台
环境
本地验证使用 package test runner，目标测试文件是
packages/core/src/tools/computer-use/tool.test.ts。风险与范围
关联 Issue
Fixes #5499
测试
AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.