tool add show_on#732
Conversation
dosubot
Bot
added
size:L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds conditional visibility to tool parameters and options through a new ShowOn field and the ToolParameterShowOnObject struct. It includes custom unmarshaling logic for JSON and YAML to handle default slice initialization and adds corresponding validation tests. Review feedback identifies non-idiomatic pointer usage in the YAML decoding process and recommends increasing validation length limits for consistency with existing parameter name constraints.
| Alias: (*Alias)(p), | ||
| } | ||
|
|
||
| if err := value.Decode(&aux); err != nil { |
There was a problem hiding this comment.
The use of &aux in value.Decode(&aux) is non-idiomatic because aux is already a pointer to the target struct. Passing a pointer to a pointer (**struct) is unnecessary and inconsistent with the json.Unmarshal(data, aux) call used in the UnmarshalJSON method above.
| if err := value.Decode(&aux); err != nil { | |
| if err := value.Decode(aux); err != nil { |
| Variable string `json:"variable" yaml:"variable" validate:"required,lt=256"` | ||
| Value string `json:"value" yaml:"value" validate:"required,lt=256"` |
There was a problem hiding this comment.
The length limit for Variable (lt=256) is inconsistent with the limit for ToolParameter.Name (lt=1024). Since Variable is intended to store a parameter name for conditional visibility, it should support the same maximum length. Similarly, Value should likely have a matching limit to ensure consistency when comparing against parameter values.
Variable string `json:"variable" yaml:"variable" validate:"required,lt=1024"`\n Value string `json:"value" yaml:"value" validate:"required,lt=1024"`| Alias: (*Alias)(t), | ||
| } | ||
|
|
||
| if err := value.Decode(&aux); err != nil { |
There was a problem hiding this comment.
The use of &aux in value.Decode(&aux) is non-idiomatic because aux is already a pointer to the target struct. Passing a pointer to a pointer (**struct) is unnecessary and inconsistent with the json.Unmarshal(data, aux) call used in the UnmarshalJSON method above.
| if err := value.Decode(&aux); err != nil { | |
| if err := value.Decode(aux); err != nil { |
1 participant
Description
Fixes langgenius/dify#36141
This pull request adds conditional visibility metadata for tool parameters and select options via
show_on, aligned with the intent of the upstream change setToolParameterShowOnObject(variable+value) and attachesshow_ontoToolParameterandParameterOption.show_onto an empty slice (non-nil) for stable consumers and validation.show_onfor both JSON and YAML (where applicable).Type of Change
Essential Checklist
Testing
Bug Fix (if applicable)
Fixes #123orCloses #123)Additional Information
pkg/entities/plugin_entities/constant.go,pkg/entities/plugin_entities/tool_declaration.go,pkg/entities/plugin_entities/tool_declaration_test.gogo test ./pkg/entities/plugin_entities/...on a toolchain satisfyinggo.mod.