fix(toolbox-langchain): avoid duplicated parameter descriptions#680
fix(toolbox-langchain): avoid duplicated parameter descriptions#680WilliamK112 wants to merge 2 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Yuan325
added
the
priority: p1
0ed0c21 to
86be7ae
Compare
|
/gcbrun |
|
|
||
|
|
||
| def _get_tool_description(core_tool: ToolboxCoreTool) -> str: | ||
| description = core_tool._description |
There was a problem hiding this comment.
in the case the tool lacks the description, would this raise an attribute error or fallback to doc ?
There was a problem hiding this comment.
It now falls back to __doc__ without raising AttributeError. I changed the lookup to getattr(core_tool, "_description", None) in both sync and async adapters, then use core_tool.__doc__ or "" when the structured description is unavailable. This is covered in 92108fd.
| sync_mock.__name__ = "test_tool_name_for_langchain" | ||
| sync_mock.__doc__ = tool_schema_dict["description"] | ||
| sync_mock._description = tool_schema_dict["description"] | ||
| sync_mock.__doc__ = ( |
There was a problem hiding this comment.
Can we add a test to check the fallback to doc
There was a problem hiding this comment.
Added fallback coverage for both adapters in 92108fd: the sync test removes _description, and the async test sets the underlying description to None; both assert that the LangChain tool uses __doc__. The focused suite passes (30 passed).
92108fd to
f439261
Compare
|
/gcbrun |
|
Triggered tests and passing it to Disha since her comments have responses :) |
Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
f439261 to
1b293f3
Compare
|
/gcbrun |
4 participants
Fixes googleapis/mcp-toolbox#1327.
LangChain tools currently use the core tool
__doc__as theBaseTooldescription. The core docstring intentionally appends anArgs:section for Python introspection, but LangChain already receives parameter descriptions throughargs_schema, so the LLM-facing tool description duplicates every parameter description.This changes the LangChain wrappers to use the core tool's raw
_descriptionwhen available, while preserving a fallback to__doc__for compatibility with older or mocked tool objects. Regression coverage checks that the LangChain tool description no longer includesArgs:while parameter descriptions remain available throughargs_schema.Validation:
pytest tests/test_client.py tests/test_async_client.py tests/test_tools.py tests/test_async_tools.pyblack --check src testsisort --check src testsMYPYPATH='./src:../toolbox-core/src' mypy --install-types --non-interactive --cache-dir=.mypy_cache/ -p toolbox_langchainNote: the full
pytest testssuite includes E2E coverage that expects a Toolbox service/version environment, so I used the non-E2E LangChain package test set above for this wrapper change.