fix: accept dict tool arguments in ChatMessage.from_openai_dict_format - #12790
Conversation
Some OpenAI-compatible servers send parsed argument objects. json.loads then raised TypeError. Invalid JSON strings are treated as an empty mapping.
|
@MohammadHijjawi97 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @MohammadHijjawi97, thanks for your interest in contributing to Haystack! 🙏 This is an automated message to help us keep the review queue healthy. |
| OpenAI sends a JSON string. Some OpenAI-compatible servers already send a dict, omit the field, or send | ||
| null or an empty string for a zero-argument call. | ||
| """ | ||
| if isinstance(raw_arguments, dict): |
There was a problem hiding this comment.
from_openai_dict_format is meant to mirror to_openai_dict_format, which always emits arguments as a JSON string. Which OpenAI-compatible server sends a parsed dict here? Our chat generators don't route responses through this method.
There was a problem hiding this comment.
Fair point. I hit this by calling from_openai_dict_format directly with arguments already parsed as a dict (some OpenAI-compatible SDKs / providers return tool call args that way). Haystack's own chat generators do not go through this method, so I do not have a production Haystack path that needs it. Happy to close this if you would rather keep the contract mirrored to to_openai_dict_format only.
|
CLA is signed now. This is ready for review whenever you have time. |
d9c2a04 to
d1b33aa
Compare
|
Fair question. I don't have a concrete server that sends a parsed dict here; this was a defensive change rather than a fix for a reported failure. Since |
|
Thanks for the PR @MohammadHijjawi97! Accepting dict arguments makes sense. I'd just like to keep raising on invalid JSON like before, so a truncated tool call like |
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
2 participants
Related Issues
No existing issue. This PR includes a regression test for the crash.
Proposed Changes:
Proposed Changes:
ChatMessage.from_openai_dict_formatalways passed tool-callargumentstojson.loads. Some OpenAI-compatible servers already send a parsed dict, which raisedTypeError. Invalid JSON strings also crashed. Accept a dict payload, treat empty/null/invalid JSON as{}.How did you test it?
Added unit tests for dict arguments and invalid JSON. Existing empty/missing argument tests still pass.
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.