Skip to content

fix(sdk): return an error to the model for malformed tool-call arguments - #1168

Merged
santoshkumarradha merged 3 commits into
Agent-Field:mainfrom
pcbeingused333:fix/tool-call-malformed-args
Nov 10, 2026
Merged

santoshkumarradha merged 3 commits into
Agent-Field:mainfrom
pcbeingused333:fix/tool-call-malformed-args

Conversation

@pcbeingused333

Copy link
Copy Markdown
Contributor

Root cause. In execute_tool_call_loop (Python, tool_calling.py) and ExecuteToolCallLoop (Go, tool_calling.go), a tool call whose arguments is not valid JSON (e.g. truncated by max_tokens) was swallowed (except JSONDecodeError: func_args = {} / json.Unmarshal error ignored) and the tool was then executed with empty arguments. The tool could run with missing or default inputs and the model never learned its call was broken. In the Python SDK, the sibling case of a missing arguments field already returns an error to the model.

Fix. Invalid, truncated, or non-object arguments now append a tool-error message (... has invalid or truncated JSON arguments (...). Please retry with valid JSON arguments.) and skip execution, in both SDKs. An empty arguments string is still treated as "no arguments", because some providers send "" for parameterless tools.

Tests. Python: test_malformed_arguments_are_not_executed (3 cases) and test_empty_string_arguments_run_with_no_args. Go: TestExecuteToolCallLoop_MalformedArgumentsAreNotExecuted; the old test that asserted a tool ran with empty input after {bad json was reworked to use an empty-string argument. The new tests fail before the change and pass after; go test ./ai/ and tests/test_tool_calling.py pass.


This PR was prepared with the help of an AI assistant (Claude Code) and submitted by a human who is responsible for it.

When an LLM tool call carried invalid or truncated JSON arguments, both the
Python and Go SDKs silently replaced them with {} and still executed the
tool, so the tool ran with missing/default inputs and the model never learned
its call was broken. Report the parse error back as a tool message (like the
existing missing-"arguments" case) and skip execution. An empty arguments
string is still accepted as "no arguments", since some providers send that for
parameterless tools.
Signed-off-by: Alex Castillo <alex.castillog33@gmail.com>
@pcbeingused333
pcbeingused333 requested review from a team and AbirAbbas as code owners November 9, 2026 10:44
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Performance

SDK Memory Δ Latency Δ Tests Status
Python 9.1 KB - 0.32 µs -9% ✓ ✓
Go 225 B -20% 0.74 µs -26% ✓ ✓

✓ No regressions detected

@santoshkumarradha santoshkumarradha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tightening malformed tool-call handling. The intent looks right, but the Python CI is red because the existing error-path tests still index trace.calls[0] after this change. Invalid arguments now correctly avoid creating a tool-call record, so please update those assertions to inspect the tool error message / traced error instead, then rerun the Python suite.

Invalid arguments no longer create a tool-call record, so assert on the
tool error message fed back to the model instead of trace.calls.
@pcbeingused333

Copy link
Copy Markdown
Contributor Author

Thanks, updated. tests/test_tool_calling_error_paths.py now asserts on the tool error message fed back to the model (and that no call record is created) instead of indexing trace.calls[0]. In an AI-assisted run of sdk/python/tests on Python 3.12: 2877 passed, 0 failed; the 26 errors were in tests that could not be set up there, such as the functional and integration ones.

This comment was drafted with Claude Code.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 88.20% 87.40% ↑ +0.80 pp 🟡
sdk-go 93.80% 92.00% ↑ +1.80 pp 🟢
sdk-python 94.73% 93.73% ↑ +1.00 pp 🟢
sdk-typescript 92.44% 90.42% ↑ +2.02 pp 🟢
web-ui 84.89% 84.79% ↑ +0.10 pp 🟡
aggregate 86.11% 85.75% ↑ +0.36 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 0 — ➖ no changes
sdk-go 21 90.00% ✅
sdk-python 0 — ➖ no changes
sdk-typescript 0 — ➖ no changes
web-ui 0 — ➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

@santoshkumarradha santoshkumarradha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the error-path coverage. The tests now assert the reported tool error without assuming a call record exists, which matches the intended malformed-argument behavior. Focused Go and Python suites pass locally.

@santoshkumarradha
santoshkumarradha added this pull request to the merge queue Nov 10, 2026
Merged via the queue into Agent-Field:main with commit bcef6b5 Nov 10, 2026
26 checks passed
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.

2 participants