add param type date and date-picker#733
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces DATE and DATE_PICKER as new tool parameter types and updates the validation logic. The review feedback points out that the existing ToolParameter struct cannot enforce date range constraints due to its float64 fields and lacks format validation for default values. Additionally, it is suggested to evaluate if both types are necessary to avoid redundancy.
| TOOL_PARAMETER_ARRAY ToolParameterType = ARRAY | ||
| TOOL_PARAMETER_OBJECT ToolParameterType = OBJECT | ||
| TOOL_PARAMETER_TYPE_CHECKBOX ToolParameterType = CHECKBOX | ||
| TOOL_PARAMETER_TYPE_DATE ToolParameterType = DATE |
There was a problem hiding this comment.
The addition of the DATE type highlights a limitation in the ToolParameter struct (line 128), where Min and Max are defined as *float64. These fields cannot be used to enforce date range constraints. Additionally, there is currently no validation for the Default value to ensure it follows a specific format (e.g., ISO 8601). Consider updating the validation logic to support these requirements for date-related types.
| TOOL_PARAMETER_OBJECT ToolParameterType = OBJECT | ||
| TOOL_PARAMETER_TYPE_CHECKBOX ToolParameterType = CHECKBOX | ||
| TOOL_PARAMETER_TYPE_DATE ToolParameterType = DATE | ||
| TOOL_PARAMETER_TYPE_DATE_PICKER ToolParameterType = DATE_PICKER |
There was a problem hiding this comment.
Consider whether both DATE and DATE_PICKER are necessary as distinct types. If they both represent a date string and the difference is purely a UI hint, it might be more maintainable to have a single DATE type. This avoids redundancy in the type system and simplifies the logic for consumers of these parameters.
Description
Fixes langgenius/dify#36162
tool plugin add type date and date-picker
Type of Change
Essential Checklist
Testing
Bug Fix (if applicable)
Fixes #123orCloses #123)Additional Information
Please provide any additional context that would help reviewers understand the changes.