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. |
* Adding scalar encoding, make tools use signature and update fixtures * Lint * Lint * More general wrappedencodable * Lint * Architectural fixes * Removing embedded_type * Lint * Minor * Minor fix * Lint * Add param_schema_type * Minor * New Encodable's architecture * Recursive dataclass encodable * Linting * Simplify encodable * Minor * Fix test * Lint * Trim * More trimming * Handling images * Lint * Further trimming * Updating test template formatting * Minor * Add MappingEncodable and cover more tests * Lint * More precise BaseEncodable * Lint * Removing adapter from base * Move wrapped model to a staticmethod * More concise handling of empty lists * Minor * Use nested_type as _wrapped_model is resolved * Remove DTC_IDS * Minor fix * Special typeddict handling * Complex Encodable * Removing casts * Add Encoding/Integration Parameterized Tests (#582) * Adding tests * Lint * More lint * Minor * Removing helper functions * Check deserialize invariant * Minor fix for typeddict * Better pydantic-based check * Lint * Test ToolSchema * Inline completion with tools * More inlining * reimport requires_openai * Removing casts * mark xfail some tests agian * Merging integration test into encoding * Enforce Tool and Image to be PydanticBase model * Update fixtures * Update minor fixture
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.