Repository navigation
fix: validate inputs_from_state when a tool has no input parameters - #12813
Open
Lesereingrape wants to merge 1 commit into
Open
Lesereingrape wants to merge 1 commit into
Lesereingrape wants to merge 1 commit into
Conversation
The check was gated on `valid_inputs` being truthy, which conflated "this tool takes no input" with "we could not resolve the inputs". For a zero-argument function, or a ComponentTool around a component without input sockets, a mapping to a name that cannot exist was accepted at construction time and only surfaced later as a ToolInvocationError that never mentions inputs_from_state. _get_valid_inputs() is typed `-> set[str]` and always returns a resolved set, so an empty set is knowledge rather than missing information; the sibling output check already distinguishes the two with a `set[str] | None` sentinel.
Lesereingrape
requested review from
julian-risch
and removed request for
a team
September 19, 2026 10:32
Contributor
|
@Lesereingrape is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
Proposed Changes:
Tool.__init__validatedinputs_from_statewithif valid_inputs and param_name not in valid_inputs:(haystack/tools/tool.py:208). Thevalid_inputs andconjunct makes "this tool has no input parameters" indistinguishable from "we could not work out the parameters", so for a zero-argument function — or aComponentToolaround a component without input sockets — a mapping to a name that cannot exist was accepted at construction time. The user then met it at the worst possible moment: inside the agent loop, asToolInvocationError: Failed to invoke Tool \clock` with parameters {'city': 'Berlin'}. Error: no_param_tool() got an unexpected keyword argument 'city', which never mentionsinputs_from_state`.The fix drops that one conjunct, so the check is
if param_name not in valid_inputs:. An empty set now validates as strictly as a non-empty one, and the error reads... Valid parameters are: set().Why the empty set is safe to validate against, and why the guard was asymmetric in the first place:
_get_valid_inputs()is typed-> set[str](tool.py:214) and always returns a set — the union of the callable's signature parameters and thepropertiesdeclared inparameters. AToolwith no parameters has resolved that set and found it empty; that is knowledge, not missing information._get_valid_outputs()(tool.py:249) is typed-> set[str] | Noneprecisely because function-based tools genuinely have no output schema, and its caller tests the sentinel withis not None(tool.py:148-149) rather than by truthiness. So outputs already distinguish "none" from "unknown"; inputs conflated the two.Tool,ComponentTool,AgentToolandPipelineToolall resolve a non-empty input set in normal use except for the no-input case this PR fixes;from_functionbuilds the schema from the same signature, so the set it reports is the set the mapping must name.Impact for users: a configuration error that previously produced a confusing runtime failure inside an agent run now fails at construction, with the valid names listed — matching what already happens for every tool that takes input.
How did you test it?
Two tests, one per affected construction path, plus the existing suites of both files.
main(b717d00, source reverted)test_tool.py::TestTool::test_inputs_from_state_validation_with_no_valid_parametersValueError)test_component_tool.py::TestComponentTool::test_from_component_with_inputs_from_state_and_no_input_socketsValueError)Both new tests carry an in-file control: the
ComponentToolone first assertstool.parameters == {"type": "object", "properties": {}}, i.e. that the empty schema really comes from a component with no input sockets rather than from a mistake in the test. The neighbouringtest_inputs_from_state_validation_*tests (non-empty valid inputs,TypeErroron non-string values) exercise the untouched branches and pass on both sides.Environment note, stated plainly:
hatchis not available on my machine, so I did not runhatch run test:unitorhatch run test:types. I ran against an editable install of this checkout (haystack 3.2.0-rc0,import haystackresolving to the clone, Python 3.13.5) withpytestdirectly, after installing the test environment's pytest plugins (pytest-bdd,pytest-asyncio,pytest-rerunfailures,pytest-cov,flaky) so that both files collect. The 7 skips are pre-existing (integration-marked) and unchanged between the two runs. For lint I ran the repository's pinned ruff (v0.16.0, the rev in.pre-commit-config.yaml) on the changed files —All checks passed!/3 files already formatted— andscripts/release_note_backticks.pyon the new release note (exit 0). I also ranmypyover the three changed files with--ignore-missing-imports --follow-imports=silent: no errors — but that is a looser configuration than the repo's[tool.mypy]pass overtest/tools/, so consider CI the first authoritative type check. Pre-commit hooks are not installed in my clone, so the individual tools above were run directly rather than throughpre-commit run --all-files;codespellwas not run.Notes for the reviewer
haystack/tools/tool.py:208; tests intest/tools/test_tool.pyandtest/tools/test_component_tool.py; release note atreleasenotes/notes/tool-empty-inputs-from-state-validation-bc8460226991e1ba.yaml.Tool/ComponentToolwith a bogusinputs_from_statemapping and never invokes it will start raising at construction. That is the bug being reported, and the tool would have failed at invocation anyway — but it is still a behavior change for such callers, and I want it visible in review rather than buried._get_valid_inputs()staysset[str]. Making itset[str] | Noneto mirror outputs would reintroduce the same silent skip for tools whose signature cannot be introspected (inspect.signatureraisingValueError/TypeError,tool.py:230), and today those tools still yield a usable set fromparameters["properties"]. If you would rather have inputs and outputs share theNonesentinel, I can rework it that way.inputs_from_statevalidation (searched title/body across open PRs). The only matching issue is inputs_from_state typos pass construction when a tool takes no input #12811, which I filed for this report. My own fix(auth): deserialize listed secrets when recursive is enabled #12810 toucheshaystack/utils/auth.pyand does not overlap with this change.Checklist
inputs_from_stateparameter docs already state that it maps state keys to the tool's input parameters.)fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.ruff0.16.0 check + format,release_note_backticks.py) and they are clean; I could not runhatchor the full hook suite locally, and mymypywas run with a looser configuration than CI's — see "How did you test it?" for the exact scope.