Skip to content

Commit 906d0e0

Browse files
authored
fix(toolbox-core): Prevent rebinding of parameters in ToolboxTool (#186)
* fix(toolbox-core): Prevent rebinding of parameters in ToolboxTool This pull request updates the `bind_parameters` helper function within the `ToolboxTool` of the `toolbox-core` package to enhance parameter binding management. * The helper now throws an explicit error if a user attempts to bind a parameter that has already been bound. This change aims to prevent unexpected behavior and improve the clarity of parameter assignments. * The error that previously occurred when a user tried to bind a parameter not present in the tool's schema has been removed. This validation will be handled by a `strict` mode implementation in a future PR. * chore: Update unit test case * chore: Delint * fix: Add the no parameter check back again. We will remove this once we actually implement the `strict` flag and centralize this functionality by moving this check to the tool's constructor in a future PR. * fix: Reverse the error conditions to avoid masking of the second error. * chore: Fix unit test cases. * chore: Make unit test more robust. * chore: Delint
1 parent b85820b commit 906d0e0

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

packages/toolbox-core/src/toolbox_core/tool.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,11 @@ def bind_parameters(
316316
"""
317317
param_names = set(p.name for p in self.__params)
318318
for name in bound_params.keys():
319+
if name in self.__bound_parameters:
320+
raise ValueError(
321+
f"cannot re-bind parameter: parameter '{name}' is already bound"
322+
)
323+
319324
if name not in param_names:
320325
raise Exception(f"unable to bind parameters: no parameter named {name}")
321326

packages/toolbox-core/tests/test_client.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -431,14 +431,38 @@ async def test_bind_callable_param_success(self, tool_name, client):
431431

432432
@pytest.mark.asyncio
433433
async def test_bind_param_fail(self, tool_name, client):
434-
"""Tests 'bind_param' with a bound parameter that doesn't exist."""
434+
"""Tests 'bind_parameters' with a bound parameter that doesn't exist."""
435435
tool = await client.load_tool(tool_name)
436436

437437
assert len(tool.__signature__.parameters) == 2
438438
assert "argA" in tool.__signature__.parameters
439439

440-
with pytest.raises(Exception):
441-
tool = tool.bind_parameters({"argC": lambda: 5})
440+
with pytest.raises(Exception) as e:
441+
tool.bind_parameters({"argC": lambda: 5})
442+
assert "unable to bind parameters: no parameter named argC" in str(e.value)
443+
444+
@pytest.mark.asyncio
445+
async def test_rebind_param_fail(self, tool_name, client):
446+
"""
447+
Tests that 'bind_parameters' fails when attempting to re-bind a
448+
parameter that has already been bound.
449+
"""
450+
tool = await client.load_tool(tool_name)
451+
452+
assert len(tool.__signature__.parameters) == 2
453+
assert "argA" in tool.__signature__.parameters
454+
455+
tool_with_bound_param = tool.bind_parameters({"argA": lambda: 10})
456+
457+
assert len(tool_with_bound_param.__signature__.parameters) == 1
458+
assert "argA" not in tool_with_bound_param.__signature__.parameters
459+
460+
with pytest.raises(ValueError) as e:
461+
tool_with_bound_param.bind_parameters({"argA": lambda: 20})
462+
463+
assert "cannot re-bind parameter: parameter 'argA' is already bound" in str(
464+
e.value
465+
)
442466

443467
@pytest.mark.asyncio
444468
async def test_bind_param_static_value_success(self, tool_name, client):

0 commit comments

Comments
 (0)