Repository navigation
fix(bedrock): parse streamed Converse tool input like the non-streamed output - #963
Conversation
…d output
converse_stream spans logged a tool_use block's input as the raw JSON
string joined from the toolUse deltas, while converse spans log the dict
Bedrock returns. Parse the joined string once the stream ends ("" becomes
{}, input that doesn't parse, e.g. cut off by maxTokens, stays a string).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd8c819a33
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| return {} | ||
| try: | ||
| return json.loads(text) | ||
| except ValueError: |
There was a problem hiding this comment.
When a Bedrock model emits a valid JSON tool argument nested roughly 1,000 levels on supported Python 3.10/3.11, json.loads raises RecursionError, which bypasses this ValueError handler. _finish invokes this after setting _finished and from the generator's else path, so the exception escapes the final next() and leaves the span unlogged even though the Bedrock stream completed successfully; decoder-limit failures should fall back to the raw string too.
AGENTS.md reference: AGENTS.md:L17-L20
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, thanks. Fixed in 9477af1: _parse_streamed_tool_input now also falls back to the raw string on RecursionError, as harbor/identity.py already does, with a unit test for a 100,000-level nested input.
…epth json.loads raises RecursionError, not ValueError, for a document nested deeper than the decoder allows, so such tool input escaped the fallback and the span was never logged. Fall back to the raw string for it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
1 participant
What
_TracedConverseStreamjoined thetoolUsedeltas of each block and stored the joined JSON string as the block'sinput. Aconverse_streamspan therefore logged{"type": "tool_use", "input": "{\"city\":\"Paris\"}"}, while the same call throughconverselogsinputas the dict Bedrock returns.test_wrap_bedrock_converse_normalizes_tool_config_and_outputalready asserts that shape for the non-streamed path (isinstance(tool_use.get("input"), dict)).The joined string is now parsed when the stream ends:
{}.maxTokens, stays the raw string.Text, reasoning and usage handling are unchanged.
Checked against real
converse_streamcaptures (Claude Haiku 4.5 / Sonnet 4.6 with thinking, Nova Lite v1, Nova 2 Lite, gpt-oss-20b, all with tool calls). Everytool_useblock was a string before and is a dict after. Reasoning text and signature, and usage, were already right.Tests
test_wrap_bedrock_converse_stream_parses_tool_input, with a cassette recorded from one realconverse_streamcall on the existingCONVERSE_STREAM_MODEL(us.amazon.nova-lite-v1:0) and_converse_tool_kwargs. It's incassettes/latest/and, like the other Bedrock cassettes, copied unchanged tocassettes/1.34.116/. It checks that the span'stool_use.inputequals the parsed streamed arguments and is a dict. It fails on main ('{"city":"Paris"}' == {'city': 'Paris'}) and passes with the change.nox -s "test_bedrock_runtime(latest)": 11 passed.nox -s "test_bedrock_runtime(1.34.116)": 10 passed, 1 skipped (the existing document-block test that 1.34.116 can't run). The new test also fails on main in the 1.34.116 session.vcr_config.This change was written with Claude Code; the tests above were run on main and on this branch.
🤖 Generated with Claude Code