Move tool/tool call encoding logic from completion to encoding#548
Move tool/tool call encoding logic from completion to encoding#548
Conversation
| includes the raw assistant message for retry handling. | ||
| """ | ||
| tool_specs = {k: _function_model(t) for k, t in tools.items()} | ||
| response_model = pydantic.create_model( |
There was a problem hiding this comment.
Removing response_model here was the original motivation for this PR because it was causing problems with less capable LLMs in #545, but I ended up doing a bit more cleanup along the way.
|
@datvo06 I refactored the |
datvo06
left a comment
datvo06
left a comment
There was a problem hiding this comment.
I have one concern with using enc directly. It's ok, but we have to make the encoding complete.
| model, | ||
| messages=list(messages), | ||
| response_format=response_model, | ||
| response_format=None if response_format.enc is str else response_format.enc, |
There was a problem hiding this comment.
response_model is changed to response_format.enc.
response_format.enc is just a python type, not asserted to be a Pydantic model (created through pydantic.create_model).
I understand that we want to remove the extra value wrapping layer.
But _encodable_object is not handling dataclass: enc = ty sets the type to raw class.
I think I can add a 1-line fix inside there and see if it works.
There was a problem hiding this comment.
Also, because we are not creating pydantic model here anymore, and instead leave that to encodable, we would need to handle primitive types as well.
datvo06
force-pushed
the
eb-remove-response
branch
from
26ff31b to
ab9097d
Compare
|
I accidentally pushed the fix. I reverted it and making a PR on top. |
2 participants
This refactoring PR cleans up some logic in
handlers.llm.completionsrelated to tool call encoding and decoding by coercing it into the existingEncodableAPI.