-
Notifications
You must be signed in to change notification settings - Fork 9.2k
fix(autogen-ext): skip LangChain callback-manager (run_manager) when inferring tool args schema #7994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(autogen-ext): skip LangChain callback-manager (run_manager) when inferring tool args schema #7994
Changes from 1 commit
ac750c4
c1b1fc2
315e4f0
7b5e64d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,3 +100,38 @@ async def test_langchain_tool_adapter(caplog: pytest.LogCaptureFixture) -> None: | |
| # Test run method for CustomCalculatorTool | ||
| custom_result = await custom_adapter.run_json({"a": 3, "b": 4}, CancellationToken()) | ||
| assert custom_result == 12 | ||
|
|
||
|
|
||
| class NoSchemaTool(LangChainTool): | ||
| name: str = "NoSchema" | ||
| description: str = "a tool without an explicit args schema" | ||
|
|
||
| def _run(self, a: int, b: int, run_manager: Optional[CallbackManagerForToolRun] = None) -> int: | ||
| return a + b | ||
|
|
||
| async def _arun( | ||
| self, | ||
| a: int, | ||
| b: int, | ||
| run_manager: Optional[AsyncCallbackManagerForToolRun] = None, | ||
| ) -> int: | ||
| return a + b | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_langchain_tool_adapter_skips_run_manager() -> None: | ||
| # Tools without an explicit args_schema get their args inferred from the | ||
| # callable's signature. LangChain injects a ``run_manager`` into ``_run`` | ||
| # which is not a user-facing input and cannot be turned into a pydantic | ||
| # schema; the adapter must skip it (see #6385). | ||
| tool = NoSchemaTool() | ||
| adapter = LangChainToolAdapter(tool) # type: ignore | ||
|
|
||
| schema = adapter.schema | ||
| assert schema["name"] == "NoSchema" | ||
| props = schema["parameters"]["properties"] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pyright reports three
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in c1b1fc2: the test now asserts parameters, properties, and required are present before indexing the optional TypedDict keys. Targeted tests and type checks pass on the current head. Please re-review. |
||
| assert set(props.keys()) == {"a", "b"} | ||
| assert set(schema["parameters"]["required"]) == {"a", "b"} | ||
|
|
||
| result = await adapter.run_json({"a": 2, "b": 3}, CancellationToken()) | ||
| assert result == 5 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we filter
callbacksby name here as well? The pinnedlangchain-coredefinesFILTERED_ARGS = ("run_manager", "callbacks"), andCallbacksis a sequence of callback handlers, so_is_callback_manager_annotationdoes not match it. I reproduced this with a no-schema tool whose_runacceptscallbacks: Callbacks = None: constructingLangChainToolAdapterraisesPydanticSchemaGenerationErrorforBaseCallbackHandler. Usingk in ("run_manager", "callbacks")and extending this regression test to includecallbacksfixes the crash; focused pytest, Pyright, mypy, and Ruff all pass.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in c1b1fc2: the inferred-schema path now filters both reserved LangChain parameters, run_manager and callbacks, and the regression suite covers the callbacks case. The current head also includes the pinned Ruff formatting fix. Please re-review.