fix(openai): treat non-dict JSON tool arguments as invalid tool calls#36055
fix(openai): treat non-dict JSON tool arguments as invalid tool calls#36055RoomWithOutRoof (Jah-yee) wants to merge 2 commits intolangchain-ai:masterfrom
Conversation
Fixes issue langchain-ai#36011 - merge_dicts raises TypeError for float values during streaming chunk aggregation The function handled str, dict, list, int, and equal values but not float. When two dicts contain the same key with unequal float values (e.g., logprobs, scores, safety scores from LLM providers), the function raised a TypeError. This fix adds float to the isinstance check alongside int, using the same last-wins strategy for temporal fields (index, created, timestamp) and summing behavior for other numeric fields.
When json.loads() parses valid JSON that is not a dict (arrays, strings, numbers, booleans, or null), the result was passed directly as args in a tool_call entry, causing ValidationError in AIMessage because ToolCall.args is typed as dict[str, Any]. This fix adds isinstance(args, dict) check after json.loads() succeeds. Non-dict results now route to invalid_tool_calls with a descriptive error message. Also catches TypeError alongside JSONDecodeError for consistency. Fixes langchain-ai#36052
Merging this PR will improve performance by 73.59%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_import_time[RunnableLambda] |
522.3 ms | 378.2 ms | +38.09% |
| ⚡ | WallTime | test_import_time[PydanticOutputParser] |
565.5 ms | 410.4 ms | +37.79% |
| ⚡ | WallTime | test_import_time[tool] |
582.2 ms | 406.1 ms | +43.36% |
| ⚡ | WallTime | test_import_time[HumanMessage] |
277.3 ms | 200.7 ms | +38.11% |
| ⚡ | WallTime | test_import_time[BaseChatModel] |
573.4 ms | 408.5 ms | +40.37% |
| ⚡ | WallTime | test_import_time[Runnable] |
522.8 ms | 379.8 ms | +37.67% |
| ⚡ | WallTime | test_import_time[Document] |
200.2 ms | 145.3 ms | +37.72% |
| ⚡ | WallTime | test_import_time[ChatPromptTemplate] |
677.3 ms | 473.6 ms | +43.02% |
| ⚡ | WallTime | test_async_callbacks_in_sync |
25.2 ms | 14.5 ms | +73.59% |
| ⚡ | WallTime | test_import_time[LangChainTracer] |
480.7 ms | 352.1 ms | +36.53% |
| ⚡ | WallTime | test_import_time[CallbackManager] |
336.1 ms | 243.1 ms | +38.23% |
| ⚡ | WallTime | test_import_time[InMemoryRateLimiter] |
181.2 ms | 135.4 ms | +33.74% |
| ⚡ | WallTime | test_import_time[InMemoryVectorStore] |
643.7 ms | 464.5 ms | +38.58% |
| 🆕 | WallTime | test_init_time |
N/A | 140.3 ms | N/A |
| 🆕 | WallTime | test_init_time |
N/A | 2.9 ms | N/A |
| 🆕 | WallTime | test_init_time |
N/A | 2.9 ms | N/A |
Comparing Jah-yee:fix/non-dict-json-tool-calls (d30a7e8) with master (07fa576)2
Footnotes
-
23 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
master(54a5f83) during the generation of this report, so 07fa576 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
|
This PR has been automatically closed because you are not assigned to the linked issue. External contributors must be assigned to an issue before opening a PR for it. Please:
|
Why
json.loads() successfully parses valid JSON that is not a dict. This fix adds isinstance check after json.loads.
Fixes #36052