Skip to content

fix(tools): preserve nested numeric argument coercions - #1038

Draft
yunyv wants to merge 2 commits into
modelscope:mainfrom
yunyv:fix/nested-number-coercion
Draft

yunyv wants to merge 2 commits into
modelscope:mainfrom
yunyv:fix/nested-number-coercion

Conversation

@yunyv

@yunyv yunyv commented Oct 30, 2026

Copy link
Copy Markdown

Change Summary

coerce_arguments() supports widening Python integers to floats when a tool declares number without integer. However, when the number is inside an object or array, the outer equality check discards the converted structure: Python considers [3] == [3.0]. This makes the same tool argument behave differently at the top level and when nested.

Preserve container identity when recursive coercion makes no changes, and use identity to propagate actual conversions. Do not mutate the caller's input. The existing numeric unions, boolean exclusions, and unchanged-argument identity behavior remain intact.

The dispatch regression runs the real ToolManager.single_call_tool() against a local tool using Pydantic v1 StrictFloat: before the fix, the nested argument fails validation; after it, the call succeeds. This is a strict Python validator compatibility issue, not a claim that JSON Schema rejects integers for number.

Related issue number

No linked issue. Checked current open issues, discussion comments, all 43 open PR file lists (including the second page for #1002), and the 100 most recently updated closed PRs. No competing nested argument-coercion fix found. This change is separate from the open memory, cron, rate-limiter, and tool-permission changes.

Validation

Base: 6d90d84e55d508fcb52ea19f3912de6f6a477e38, Python 3.12.

  • New regressions before the fix: 5 failed, 5 passed. After: 10 passed.
  • Existing and new argument-coercion tests: 23 passed.
  • Related offline tool suite: baseline 5 failed, 73 passed; fixed 78 passed (same tests).
  • Additional local regression checks: 17 cases pass (baseline 7 failed / 10 passed), including nested schema and unchanged-container identity controls.
  • Scoped pre-commit hooks, compilation, and git diff --check: passed.
  • Full-repository pre-commit checks were run on separate baseline and fixed copies. Both fail on existing formatting/lint problems; the same 69 lint diagnostics appear in both. The full-check box below is deliberately unchecked.
  • No configured SDK mypy/pyright check was found. Live MCP/provider tests and the complete application suite were not run; no model calls, provider credentials, or downloads were needed.

Reproduction commands

python -m pytest tests/tools/test_arg_coercion_nested.py -q
python -m pytest tests/tools/test_arg_coercion.py tests/tools/test_arg_coercion_nested.py -q
python -m pytest tests/tools/test_arg_coercion.py tests/tools/test_arg_coercion_nested.py tests/tools/test_tool_budget.py tests/tools/test_tool_manager_timeout.py tests/tools/test_tool_name_and_reread.py tests/tools/test_local_code_executor_windows.py tests/tools/test_local_code_execution_removed.py tests/tools/test_shell_environment.py tests/tools/test_todolist_lock_dir.py -q
pre-commit run --files ms_agent/tools/arg_coercion.py tests/tools/test_arg_coercion_nested.py

Checklist

  • The pull request title is a good summary of the changes
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check
  • Documentation reflects the changes where applicable (internal identity contract clarified)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant