fix(aws): deserialize tool_use.input from string to dict in _parse_response - #905
Conversation
…drock_to_lc When Bedrock Converse API returns tool_use blocks, the input field may sometimes be a JSON string instead of a dict. This causes downstream failures when checkpointed messages are restored and sent back to Bedrock, which rejects with ValidationException. The reverse direction (_lc_content_to_bedrock) already handles this via parse_partial_json. This applies the same defense to _bedrock_to_lc for both tool_use and server_tool_use blocks.
Lucas Kim (kimnamu)
left a comment
There was a problem hiding this comment.
Thanks Amine AIT EL HARRAJ (@AmineIzanami) for this fix, and for the thorough write-up and tests — really nice to see the round-trip test in particular. I'm not a maintainer, just another Bedrock user who reviewed this carefully and ran the branch locally; sharing verified findings to help move it forward since it's been quiet for a while. Everything below is backed by actually running the code on the current main.
1. The fix is correct and the bug is real — but the symptom is even more concrete than the PR states. On current main, a tool_use block with a string input doesn't just risk a downstream Bedrock ValidationException — it raises immediately inside _parse_response, because _extract_tool_calls passes the string straight into AIMessage(tool_calls=...) and pydantic rejects it:
pydantic_core._pydantic_core.ValidationError: 1 validation error for AIMessage
tool_calls.0.args
Input should be a valid dictionary [type=dict_type, input_value='{"city": "Paris"}', input_type=str]
With this PR applied, the same input parses cleanly (content[].input and tool_calls[].args both become {"city": "Paris"}). Might be worth leading the PR description with this local crash — it's a stronger, easier-to-reproduce justification than the downstream ValidationException.
2. The streaming/_parse_response split is the right call — and #880 already covers the streaming round-trip. I confirmed this: after merging stream chunks, content[].tool_use.input stays a string by design, but the already-merged #880 (parse_partial_json in _lc_content_to_bedrock) converts it to a dict on the way back to Bedrock. So this PR correctly targets only the non-streaming parse path. Streaming yields an AIMessageChunk (which tolerates string args), so it never hits the pydantic crash above. Your reasoning in the code comment holds up.
3. Relationship to #904 / issue #827 (context for maintainers): there are now three layers touching this:
- #880 (merged) — serialization path (
_lc_content_to_bedrock), fixes the Bedrock round-trip for already-string inputs (incl. streamed messages). - #904 (open) — checkpoint-load sanitization in
langgraph-checkpoint-aws. - #905 (this) — the parse path (
_parse_response), the most upstream fix: it stops the bad string from ever entering anAIMessage. This is arguably the root-level complement to #880, and it makes #904's defense largely redundant for the Bedrock-Converse path.
4. One untested edge — non-empty invalid JSON still raises. The if tool_input else {} guard handles empty string and truncated JSON ('{"city":' → {} via parse_partial_json), which I verified. But a non-empty, non-JSON string still raises JSONDecodeError:
parse_partial_json("not json") -> JSONDecodeError: Expecting value: line 1 column 1
This is identical to the merged #880's behavior, and a malformed-but-complete input from a non-streaming response shouldn't really happen, so I don't think it's blocking — just flagging it so it's a conscious decision.
5. server_tool_use is fixed but untested. Your loop correctly handles server_tool_use (I verified a string input parses to a dict), but the 6 tests only cover tool_use. Since server_tool_use isn't extracted into tool_calls, it can't trigger the pydantic crash — but the content-side fix is still worth a regression guard. Suggested addition (verified passing locally):
def test__parse_response_server_tool_use_string_input_parsed() -> None:
"""server_tool_use string input should also be parsed to a dict."""
response = _make_bedrock_response(
[
{
"serverToolUse": {
"toolUseId": "srv_1",
"name": "web_search",
"input": '{"query": "weather in Paris"}',
}
}
]
)
msg = _parse_response(response)
assert isinstance(msg.content, list)
assert msg.content[0]["type"] == "server_tool_use"
assert msg.content[0]["input"] == {"query": "weather in Paris"}
assert isinstance(msg.content[0]["input"], dict)6. Heads-up: this needs a rebase. The branch predates a few main changes. Most relevant: #1111 (released in 1.6.0) added inline_reasoning_tags handling to _parse_response, so the lines your hunk anchors on (lc_content = _bedrock_to_lc(...) immediately followed by tool_calls = _extract_tool_calls(...)) are no longer adjacent. After rebasing, the new loop should go just before tool_calls = _extract_tool_calls(lc_content) (i.e. after the inline-reasoning expansion). I applied it there locally and the full unit suite passes:
276 passed, 1 xfailed, 1 xpassed
All checks passed! (ruff)
Thanks again — this is a clean, well-scoped fix and I'd love to see it land.
Disclosure: I reviewed this with the assistance of an AI agent (Claude Code); a human verified the reproductions, test runs, and findings above before posting.
Description
When the Bedrock Converse API returns
tool_useblocks, theinputfield may occasionally be a JSON string instead of a dict. This causes downstream failures when:ValidationException: toolUse.input must be a JSON objectThe reverse direction (
_lc_content_to_bedrock) already handles this case usingparse_partial_json(lines 2056-2057). This PR applies the same defense to_parse_response, ensuringtool_use.inputandserver_tool_use.inputare always dicts in the returnedAIMessage.Why
_parse_responseand not_bedrock_to_lc?_bedrock_to_lcis shared between full (invoke) responses and streaming deltas. During streaming,tool_use.inputarrives as partial JSON string chunks (e.g.'{"city":',' "Paris"}') that must accumulate before parsing. Placing the fix in_parse_responsetargets only the non-streaming path where input should always be a complete, parseable value.Error this fixes
Tests
6 new unit tests covering:
tool_calls[].argsis always a dictAll 624 upstream unit tests pass with no regressions.
AI Disclosure
This contribution was developed with the assistance of AI agents for code analysis, test generation, and validation.