[side-effect] The tool message dump is incomplete#4299
[side-effect] The tool message dump is incomplete#4299lvhan028 merged 2 commits intoInternLM:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a regression where OpenAI-compatible tool definitions lose the parameters payload when being dumped/serialized.
Changes:
- Adjustes the Pydantic type of
Function.parametersto allow preserving arbitrary JSON-schema-like objects in dumps.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lmdeploy/serve/openai/protocol.py
Outdated
| description: Optional[str] = Field(default=None, examples=[None]) | ||
| name: str | ||
| parameters: Optional[BaseModel] = None | ||
| parameters: Optional[object] = None |
There was a problem hiding this comment.
parameters is typed as object, which is overly permissive and allows non-mapping JSON values (e.g., a string/array) to pass validation even though OpenAI tool parameters is expected to be a JSON Schema object. Consider changing this to Optional[Dict[str, Any]] (or Optional[Mapping[str, Any]]) so request validation and generated OpenAPI schema more accurately reflect the expected shape while still preserving the full dict on model_dump().
| parameters: Optional[object] = None | |
| parameters: Optional[Dict[str, Any]] = None |
lmdeploy/serve/openai/protocol.py
Outdated
| description: Optional[str] = Field(default=None, examples=[None]) | ||
| name: str | ||
| parameters: Optional[BaseModel] = None | ||
| parameters: Optional[object] = None |
There was a problem hiding this comment.
This change fixes a regression around tool parameters being dropped during dumps, but there doesn’t appear to be a unit test covering ChatCompletionRequest parsing + Tool/Function.model_dump() retaining nested parameters. Please add a regression test that constructs a request with tools[0].function.parameters set to a nested JSON schema dict and asserts it survives a model_dump()/model_dump_json() round-trip.
|
Thank you for fixing my mistakes! |
Fix side effect brought by #4251
In the following example, the "parameters" part of
toolsis missed from the dump dict.