accept dict param schema #176
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request modifies the function calling system to accept dictionary-based parameter schemas instead of requiring Pydantic model classes. The change allows for more flexible schema definition by supporting JSON Schema dictionaries alongside the existing Pydantic BaseModel approach.
Key changes:
- Replaced Pydantic model class with dictionary schema for parameter definition
- Enhanced parameter parsing to dynamically create BaseModel instances from dictionary schemas
- Updated function handlers to work with generic BaseModel parameters
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/ai-test/src/handlers/function_calling.py | Converted from Pydantic class to dictionary schema and updated handler to use generic parameter access |
| packages/openai/src/microsoft/teams/openai/function_utils.py | Enhanced parsing logic to support dictionary schemas by dynamically creating BaseModel classes |
| default = ... if name in required else None | ||
| attrs[name] = Field(default=default, description=details.get("description", "")) | ||
|
|
||
| DynamicModel = type("DynamicParams", (BaseModel,), {"__annotations__": annotations, **attrs}) |
There was a problem hiding this comment.
Using type() to dynamically create classes can be hard to debug and understand. Consider using create_model() from Pydantic which was already imported and provides better error handling and validation features.
| "properties": {"pokemon_name": {"type": "string"}}, | ||
| "required": ["pokemon_name"], | ||
| }, | ||
| handler=pokemon_search_handler, # type: ignore |
There was a problem hiding this comment.
The type ignore comment indicates a type mismatch. Consider updating the handler signature or using proper typing to resolve this issue instead of suppressing the error.
| pokemon_name = getattr(params, "pokemon_name", None) | ||
| if not pokemon_name: | ||
| raise ValueError("Missing required parameter 'pokemon_name'") |
There was a problem hiding this comment.
Using getattr() with manual validation duplicates the validation that Pydantic models provide automatically. This approach is more error-prone and less maintainable than using a proper schema validation.
|
@MehakBindra I'm gonna close this b/c it was fixed in #179 |
No description provided.