feat: Component tool state validation #10256
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
vblagoje
added
the
ignore-for-release-notes
github-actions
bot
added
topic:tests
type:documentation
Pull Request Test Coverage Report for Build 20304787960Details
💛 - Coveralls |
vblagoje
removed
the
ignore-for-release-notes
|
@sjrl this is the best approach I found to support validation of state kys from Tool side including ComponentTool. LMK and we can adjust if needed |
|
@vblagoje taking a look now! I wanted to ask could we also add a test for |
| f"Valid outputs are: {valid_outputs}." | ||
| ) | ||
|
|
||
| def _get_valid_inputs(self) -> set[str]: |
There was a problem hiding this comment.
Would this and _get_valid_outputs also need to be implemented for MCPTool?
Or would we need to turn of this validation for MCPTool if eager_connect is False since we wouldn't know what the inputs or outputs of the MCPTool would be at that stage right?
There was a problem hiding this comment.
Right, exactly, in MCPTool we would check only when eager is true. Otherwise we simply return None to bypass this validation
Nice catch will do that as well! And add it to this PR |
|
Updated to 3.10 and added PipelineTool tests, please see if I missed something @sjrl |
| # Validate that outputs_to_state source keys exist as valid tool outputs | ||
| if self.outputs_to_state is not None: | ||
| valid_outputs: set[str] | None = self._get_valid_outputs() | ||
| if valid_outputs is not None: | ||
| for state_key, config in self.outputs_to_state.items(): | ||
| source = config.get("source") | ||
| if source is not None and source not in valid_outputs: | ||
| raise ValueError( | ||
| f"outputs_to_state: '{self.name}' maps state key '{state_key}' to unknown output '{source}'" | ||
| f"Valid outputs are: {valid_outputs}." | ||
| ) |
There was a problem hiding this comment.
Could we move this under line 94 since there we are already doing some validation on outputs_to_state. So I think it makes sense to group these two together.
Why
Prevents runtime errors from typos in
inputs_from_stateandoutputs_to_stateby validating parameter/output names at tool construction time.inputs_from_state,outputs_to_state#9423 (and more as asked to explore by @sjrl )What
_get_valid_inputs()and_get_valid_outputs()methods to baseToolclass for validation hooksTool.__post_init__()to validateinputs_from_stateagainst valid parameters andoutputs_to_stateagainst valid outputsComponentTooloverrides validation methods to return component input/output socket namesHow can it be used
How did you test it
Notes for the reviewer
Validation is DRY - all logic in base
Tool.__post_init__(). Subclasses only override_get_valid_inputs()/_get_valid_outputs()to provide their valid parameter/output sets. Function introspection + schema union ensures robustness for all tool types (function-based, ComponentTool, PipelineTool).