Skip to content

Commit 156f007

Browse files
DeanChensjcopybara-github
DeanChensj
authored and
copybara-github
committed
refactor(workflow): deduplicate callable invocation, parameter resolution, and HITL helpers
Consolidate duplicated execution and Human-in-the-Loop (HITL) plumbing across `FunctionNode`, `_ToolNode`, `FunctionTool`, and `McpTool`: - Move `_is_async_callable` and `_invoke_callable` into `utils/_sync_runner.py` so `FunctionNode`, `FunctionTool`, and `McpTool` share the same sync/async callable dispatcher. - Fix async callable handling: unwrap `functools.partial` around async callable instances via `utils._sync_runner._invoke_callable` instead of returning an unawaited coroutine object, and await awaitable return values from sync callable wrappers. - Extract `FunctionNode._resolve_parameters` and `_ResolvedParameters` to eliminate duplicate parameter lookup and coercion loops between `_bind_parameters` and `_validate_input_data`, ensuring `node_input` parameters are validated and coerced once per invocation. - Extract `auth_interrupt_id`, `apply_confirmation_resume`, and `create_confirmation_request_input` into `workflow/utils/_workflow_hitl_utils.py` and use them in both `FunctionNode` and `_ToolNode`. Co-authored-by: Shangjie Chen <[email protected]> PiperOrigin-RevId: 999945146
1 parent 9a5510e commit 156f007

10 files changed

Lines changed: 475 additions & 174 deletions

File tree

‎src/google/adk/tools/function_tool.py‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@
3535
from ..features import is_feature_enabled
3636
from ..utils import _schema_utils
3737
from ..utils._callable_utils import CallableSpec
38-
from ..utils._sync_runner import _SYNC_CALLABLE_RUNNER
38+
from ..utils._callable_utils import unwrap_callable
39+
from ..utils._sync_runner import _invoke_callable
3940
from ..utils._sync_runner import _use_sync_callable_runner as _use_sync_callable_runner
4041
from ..utils.variant_utils import GoogleLLMVariant
4142
from ._automatic_function_calling_util import build_function_declaration
@@ -325,13 +326,21 @@ def _is_invocation_type_error(
325326
return True
326327

327328
# For callables without Python code objects (e.g. C builtins like bin):
329+
# Traceback terminates in utils._sync_runner._invoke_callable (or
330+
# FunctionTool._invoke_callable / invoke / run_async). We check both the
331+
# code object and frame function name to confirm the TypeError originated at
332+
# the invocation call-site.
328333
curr = tb
329334
while curr.tb_next:
330335
curr = curr.tb_next
331-
return curr.tb_frame.f_code.co_name in (
332-
"_invoke_callable",
333-
"invoke",
334-
"run_async",
336+
return (
337+
curr.tb_frame.f_code is _invoke_callable.__code__
338+
or curr.tb_frame.f_code.co_name
339+
in (
340+
"_invoke_callable",
341+
"invoke",
342+
"run_async",
343+
)
335344
)
336345

337346
@override
@@ -491,20 +500,12 @@ async def _invoke_callable(
491500
self, target: Callable[..., Any], args_to_call: dict[str, Any]
492501
) -> Any:
493502
"""Invokes a callable, handling both sync and async cases."""
494-
495-
# Functions are callable objects, but not all callable objects are functions
496-
# checking coroutine function is not enough. We also need to check whether
497-
# Callable's __call__ function is a coroutine function
498-
is_async = inspect.iscoroutinefunction(target) or (
499-
hasattr(target, "__call__")
500-
and inspect.iscoroutinefunction(target.__call__)
503+
unwrapped = (
504+
self._spec.unwrapped_func
505+
if target is self.func
506+
else unwrap_callable(target)
501507
)
502-
if is_async:
503-
return await target(**args_to_call)
504-
runner = _SYNC_CALLABLE_RUNNER.get()
505-
if runner is not None:
506-
return await runner(target, args_to_call)
507-
return target(**args_to_call)
508+
return await _invoke_callable(target, args_to_call, unwrapped=unwrapped)
508509

509510
def _get_mandatory_args(
510511
self,

‎src/google/adk/tools/mcp_tool/mcp_tool.py‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,12 @@
4848
from ...flows.llm_flows.context._fencing import fence_schema_descriptions
4949
from ...flows.llm_flows.context._fencing import fence_tool_description
5050
from ...flows.llm_flows.context._fencing import TOOL_DESCRIPTION_PREAMBLE
51+
from ...utils._callable_utils import unwrap_callable
5152
from ...utils._function_call_names import REQUEST_CONFIRMATION_FUNCTION_CALL_NAME
5253
from ...utils._function_call_names import REQUEST_EUC_FUNCTION_CALL_NAME
5354
from ...utils._function_call_names import REQUEST_INPUT_FUNCTION_CALL_NAME
55+
from ...utils._sync_runner import _invoke_callable
56+
from ...utils._sync_runner import _is_async_callable
5457
from ...utils.context_utils import find_context_parameter
5558
# `is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING)` gates the
5659
# error-boundary and transport-crash-detection behavior added in this module.
@@ -181,19 +184,6 @@ def _read_field(model: Any, *names: str) -> Any:
181184
)
182185

183186

184-
def _is_async_callable(target: Any) -> bool:
185-
"""Whether calling ``target`` returns a coroutine.
186-
187-
Functions are callable objects, but not all callable objects are functions:
188-
``iscoroutinefunction`` is False for an instance whose ``__call__`` is async,
189-
so check that too.
190-
"""
191-
return inspect.iscoroutinefunction(target) or (
192-
hasattr(target, "__call__")
193-
and inspect.iscoroutinefunction(target.__call__)
194-
)
195-
196-
197187
class ProgressFnT(Protocol):
198188
"""The call signature a progress callback must have.
199189
@@ -510,11 +500,9 @@ async def _invoke_callable(
510500
self, target: Callable[..., Any], args_to_call: dict[str, Any]
511501
) -> Any:
512502
"""Invokes a callable, handling both sync and async cases."""
513-
514-
if _is_async_callable(target):
515-
return await target(**args_to_call)
516-
else:
517-
return target(**args_to_call)
503+
return await _invoke_callable(
504+
target, args_to_call, unwrapped=unwrap_callable(target)
505+
)
518506

519507
def _prepare_callable_args(
520508
self,

‎src/google/adk/utils/_sync_runner.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
from contextlib import contextmanager
2727
import contextvars
28+
import inspect
2829
from typing import Any
2930
from typing import Awaitable
3031
from typing import Callable
@@ -52,3 +53,37 @@ def _use_sync_callable_runner(
5253
yield
5354
finally:
5455
_SYNC_CALLABLE_RUNNER.reset(token)
56+
57+
58+
def _is_async_callable(target: Any) -> bool:
59+
"""Returns whether calling ``target`` returns a coroutine."""
60+
return inspect.iscoroutinefunction(target) or (
61+
hasattr(target, "__call__")
62+
and inspect.iscoroutinefunction(target.__call__)
63+
)
64+
65+
66+
async def _invoke_callable(
67+
target: Callable[..., Any],
68+
kwargs: dict[str, Any],
69+
*,
70+
unwrapped: Callable[..., Any] | None = None,
71+
) -> Any:
72+
"""Invokes a sync or async callable, using the sync runner when configured.
73+
74+
Note:
75+
The function name `_invoke_callable` is part of the traceback frame contract
76+
checked by `FunctionTool._is_invocation_type_error` to detect call-site
77+
TypeErrors for callables without Python code objects (e.g. C builtins).
78+
"""
79+
check_target = unwrapped or target
80+
if _is_async_callable(check_target):
81+
return await target(**kwargs)
82+
runner = _SYNC_CALLABLE_RUNNER.get()
83+
if runner is not None:
84+
result = await runner(target, kwargs)
85+
else:
86+
result = target(**kwargs)
87+
if inspect.isawaitable(result):
88+
return await result
89+
return result

0 commit comments

Comments
 (0)