feat: support for tools in OpenAIChatGenerator
#57
Conversation
anakin87
changed the title
OpenAIChatGenerator
Pull Request Test Coverage Report for Build 10451155974Details
💛 - Coveralls |
| if not text and not tool_calls: | ||
| raise ValueError("At least one of `text` or `tool_calls` must be provided.") |
There was a problem hiding this comment.
Empty messages, although uncommon, should be allowed.
There was a problem hiding this comment.
Under what circumstances can we expect empty messages from the LLM? And what makes them non-exceptional?
There was a problem hiding this comment.
we have empty assistant messages in case of content filtering: https://platform.openai.com/docs/api-reference/chat/object#chat/object-choices (see choices > finish_reason)
we handle this circumstance here: https://github.com/deepset-ai/haystack/blob/c4be8370c624c9217f34e996abd7f1d147c17c2d/haystack/components/generators/chat/openai.py#L365
| try: | ||
| arguments = json.loads(arguments_str) | ||
| except json.JSONDecodeError: | ||
| arguments = {"_malformed_json": arguments_str} | ||
| logger.warning( | ||
| "OpenAI returned a malformed JSON string. " | ||
| "You can find it under the `_malformed_json` key of `ToolCall.arguments`." |
There was a problem hiding this comment.
Important
OpenAI returns a JSON string that can be malformed. Other providers (Anthropic, Ollama) return a dict.
The current solution is just a placeholder.
How do we want to handle this?
- create an
InvalidToolCallcontent type in theChatMessage - add a
valid(or similar) bool attribute toToolCall
There was a problem hiding this comment.
If it's malformed, then we just bail and (re-)raise with some extra context - I don't see how we can handle such exceptions.
There was a problem hiding this comment.
We can start with an approach similar to what you suggest, then see if users ask to return the invalid JSON string...
Since there may be more than one tool call and some of them may be valid, I would prefer to skip the invalid ones and emit a clear warning (instead of raising an exception). I will try this approach.
anakin87
requested review from
davidsbatista and
shadeMe
and removed request for
a team
| try: | ||
| arguments = json.loads(arguments_str) | ||
| except json.JSONDecodeError: | ||
| arguments = {"_malformed_json": arguments_str} | ||
| logger.warning( | ||
| "OpenAI returned a malformed JSON string. " | ||
| "You can find it under the `_malformed_json` key of `ToolCall.arguments`." |
There was a problem hiding this comment.
If it's malformed, then we just bail and (re-)raise with some extra context - I don't see how we can handle such exceptions.
| if not text and not tool_calls: | ||
| raise ValueError("At least one of `text` or `tool_calls` must be provided.") |
There was a problem hiding this comment.
Under what circumstances can we expect empty messages from the LLM? And what makes them non-exceptional?
Related Issues
OpenAIChatGeneratorhaystack#8178Proposed Changes:
I'm creating a new version of the
OpenAIChatGeneratorto support tools (in__init__andrunmethods).Supporting the new
ChatMessageandToolrequired several changes, so I decided to completely override the original Haystack component, without altering its overall structure.functions(deprecated months ago by OpenAI in favor of tools).How did you test it?
Updated tests, new tests
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.