Suggestion (non-blocking): There's a gap in tool_arg_validator coverage when tool_name=None and none of the tool calls contain the target arg_name. The production code silently returns True in that case (the for loop finishes without failing), which is arguably a latent bug. A test documenting this would be useful:
def test_tool_arg_validator_no_tool_name_arg_missing_everywhere():
ctx = _ctx_with_tool_calls({"tool_a": _make_tool_call("tool_a", {"y": 5})})
req = tool_arg_validator("x must be positive", tool_name=None, arg_name="x", validation_fn=lambda v: v > 0)
result = req.validation_fn(ctx)
assert result.as_bool() is True # documents current (possibly surprising) behavior
Could be a follow-up issue too if you'd rather not expand scope here.
While you're here: there's a typo "Valiudation" in tool_reqs.py (appears twice in error reason strings around lines 109 and 119). Not introduced by this PR, but easy to fix if you're touching this area.
Originally posted by @psschwei in #820 (review)
Suggestion (non-blocking): There's a gap in
tool_arg_validatorcoverage whentool_name=Noneand none of the tool calls contain the targetarg_name. The production code silently returnsTruein that case (theforloop finishes without failing), which is arguably a latent bug. A test documenting this would be useful:Could be a follow-up issue too if you'd rather not expand scope here.
While you're here: there's a typo
"Valiudation"intool_reqs.py(appears twice in error reason strings around lines 109 and 119). Not introduced by this PR, but easy to fix if you're touching this area.Originally posted by @psschwei in #820 (review)