Repository navigation
Conversation
From v1.0, a function call whose function the renderer does not implement in the catalog the call resolves to (undeclared, or declared schema-only) is sent to the agent with callAgentFunction, as the specification's fallback routing defines, when the surface has an agent function caller. MessageProcessor installs one on v1.0 surfaces when it has an outbound_listener. The outbound call names the resolved catalog. Schema-only functions have their arguments validated, defaults filled in and requires_user_activation enforced before sending. The value is a pending signal (None) until the response; agent errors and timeouts are EXPRESSION_ERRORs. Identical in-flight calls share one request, and calls taking a pending agent result as an argument run once it settles. Unresolvable catalogs stay CATALOG_ERRORs and are not routed. Refs #3415, #3536 Signed-off-by: Nan Yu <nanyu@google.com>
There was a problem hiding this comment.
Code Review
This pull request implements fallback routing of unimplemented functions to the agent using callAgentFunction on v1.0 surfaces. It introduces the AgentFunctionCall and AgentFunctionCaller types, adds support for tracking in-flight agent calls to share identical requests, validates schema-only function arguments, and enforces user activation requirements. Comprehensive unit tests have been added to verify this fallback routing behavior. There are no review comments, so I have no additional feedback to provide.
…heck Signed-off-by: Nan Yu <nanyu@google.com>
…t calls in Python Signed-off-by: Nan Yu <nanyu@google.com>
… into HEAD Signed-off-by: Nan Yu <nanyu@google.com>
…t dynamic values Signed-off-by: Nan Yu <nanyu@google.com>
… into fix/python-agent-function-routing Signed-off-by: Nan Yu <nanyu@google.com> # Conflicts: # python/a2ui_core/CHANGELOG.md # python/a2ui_core/src/a2ui/core/resolution/data_context.py # python/a2ui_core/src/a2ui/core/state/surface_model.py
…agent function Signed-off-by: Nan Yu <nanyu@google.com>
gspencergoog
left a comment
There was a problem hiding this comment.
This pull request implements v1.0 fallback routing for unimplemented and schema-only catalog functions in python/a2ui_core via callAgentFunction. SurfaceModel adds in-flight request deduplication, a 256-entry LRU settled-answer cache, and deferred dispatch_action waiting, while DataContext adds _PendingResult propagation across bindings, nested arguments, and actions.
Seven high-severity defects block merging as-is. DataContext omits the allowed_callers == "agentOnly" guard, validates raw_args instead of resolved args for schema-only functions, loses activation flags on deferred local calls, marks pending Checkable rules invalid before agentFunctionResponse arrives, and re-sends failed static calls on sibling @path edits; SurfaceModel.call_agent_function also ignores self._disposed and abort_signal and raises RuntimeError without an active event loop. Four medium and two low findings cover action-context failure halting, streaming Signal forwarding, mutable cache aliasing, caller reassignment, and CHANGELOG.md accuracy.
🤖 Agent-generated
| if self._routes_to_agent(fn): | ||
| return self._call_agent_function( | ||
| name, | ||
| raw_args, | ||
| target_catalog, | ||
| fn, | ||
| abort_signal, | ||
| is_action=is_action or self._in_action_execution, | ||
| is_user_activated=effective_user_activated, | ||
| ) |
There was a problem hiding this comment.
_execute_function and _call_agent_function never check fn.allowed_callers == "agentOnly" on v1.0 surfaces, unlike web_core and dart/a2ui_core. Both schema-only and locally registered agentOnly functions remain callable from bindings, checks, and actions. Payloads can invoke restricted agentOnly functions or send unauthorized callAgentFunction RPCs instead of failing with EXPRESSION_ERROR.
| if self._routes_to_agent(fn): | |
| return self._call_agent_function( | |
| name, | |
| raw_args, | |
| target_catalog, | |
| fn, | |
| abort_signal, | |
| is_action=is_action or self._in_action_execution, | |
| is_user_activated=effective_user_activated, | |
| ) | |
| if ( | |
| self.at_least_v10 | |
| and getattr(fn, "allowed_callers", None) == "agentOnly" | |
| ): | |
| raise A2uiExpressionError( | |
| f"Function '{name}' cannot be called by renderer" | |
| " (allowedCallers is agentOnly)." | |
| ) | |
| if self._routes_to_agent(fn): | |
| return self._call_agent_function( | |
| name, | |
| raw_args, | |
| target_catalog, | |
| fn, | |
| abort_signal, | |
| is_action=is_action or self._in_action_execution, | |
| is_user_activated=effective_user_activated, | |
| ) |
🤖 Agent-generated
| try: | ||
| validated = PayloadValidator(catalog=target_catalog).validate_function( | ||
| name, raw_args | ||
| ) | ||
| except A2uiValidationError as e: | ||
| raise A2uiExpressionError( | ||
| f"Validation failed for function '{name}': {e}", | ||
| details=e.details, | ||
| ) from e | ||
| args_schema = _function_args_schema(target_catalog, fn) | ||
| written = raw_args if isinstance(raw_args, dict) else {} | ||
| defaults = _schema_defaults(args_schema) | ||
| # Validating against a Pydantic model fills in its defaults. | ||
| defaults.update({ | ||
| k: v | ||
| for k, v in (validated or {}).items() | ||
| if v is not None and k not in written | ||
| }) | ||
|
|
||
| bypass_cache = is_action or self._in_action_execution | ||
|
|
||
| def _send(resolved_args: Any) -> _PendingResult: | ||
| args: dict[str, Any] = ( | ||
| dict(resolved_args) if isinstance(resolved_args, dict) else {} | ||
| ) | ||
| if fn is not None: | ||
| args = coerce_validation_results(args_schema, args) | ||
| for key, value in defaults.items(): | ||
| args.setdefault(key, copy.deepcopy(value)) |
There was a problem hiding this comment.
_call_agent_function validates raw_args before @path and nested @call expressions settle, and _send never validates args. Bound paths holding wrong types pass pre-validation, while raw JSON schemas reject valid @path bindings. Invalid resolved arguments reach the agent over callAgentFunction, and valid bound arguments against JSON schemas fail locally.
| try: | |
| validated = PayloadValidator(catalog=target_catalog).validate_function( | |
| name, raw_args | |
| ) | |
| except A2uiValidationError as e: | |
| raise A2uiExpressionError( | |
| f"Validation failed for function '{name}': {e}", | |
| details=e.details, | |
| ) from e | |
| args_schema = _function_args_schema(target_catalog, fn) | |
| written = raw_args if isinstance(raw_args, dict) else {} | |
| defaults = _schema_defaults(args_schema) | |
| # Validating against a Pydantic model fills in its defaults. | |
| defaults.update({ | |
| k: v | |
| for k, v in (validated or {}).items() | |
| if v is not None and k not in written | |
| }) | |
| bypass_cache = is_action or self._in_action_execution | |
| def _send(resolved_args: Any) -> _PendingResult: | |
| args: dict[str, Any] = ( | |
| dict(resolved_args) if isinstance(resolved_args, dict) else {} | |
| ) | |
| if fn is not None: | |
| args = coerce_validation_results(args_schema, args) | |
| for key, value in defaults.items(): | |
| args.setdefault(key, copy.deepcopy(value)) | |
| try: | |
| PayloadValidator( | |
| catalog=target_catalog | |
| )._validate_function_identifiers(name, raw_args) | |
| except A2uiValidationError as e: | |
| raise A2uiExpressionError( | |
| f"Validation failed for function '{name}': {e}", | |
| details=e.details, | |
| ) from e | |
| args_schema = _function_args_schema(target_catalog, fn) | |
| defaults = _schema_defaults(args_schema) | |
| bypass_cache = is_action or self._in_action_execution | |
| def _send(resolved_args: Any) -> _PendingResult: | |
| args: dict[str, Any] = ( | |
| dict(resolved_args) if isinstance(resolved_args, dict) else {} | |
| ) | |
| if fn is not None: | |
| args = coerce_validation_results(args_schema, args) | |
| for key, value in defaults.items(): | |
| args.setdefault(key, copy.deepcopy(value)) | |
| try: | |
| validated = PayloadValidator( | |
| catalog=target_catalog | |
| ).validate_function(name, args) | |
| except A2uiValidationError as e: | |
| raise A2uiExpressionError( | |
| f"Validation failed for function '{name}': {e}", | |
| details=e.details, | |
| ) from e | |
| for k, v in (validated or {}).items(): | |
| if v is not None and k not in args: | |
| args[k] = copy.deepcopy(v) |
🤖 Agent-generated
| caller = self.agent_function_caller | ||
| if caller is None: | ||
| raise A2uiExpressionError( | ||
| f"Function '{call.call}' is not implemented locally and surface" | ||
| f" '{self.id}' cannot call the agent." | ||
| ) |
There was a problem hiding this comment.
call_agent_function documents and accepts abort_signal yet ignores it in the body, and omits an entry self._disposed check. Aborted signals still invoke caller(call), and disposed surfaces raise a missing-caller A2uiExpressionError. Aborted evaluations dispatch unnecessary callAgentFunction RPCs, and callers on disposed surfaces get the wrong error code.
Import A2uiRpcError and RpcErrorCode at module top, raise CANCELLED when self._disposed is True, and return a settled None future when abort_signal.aborted is True.
🤖 Agent-generated
| if key is not None: | ||
| if key in self._settled_agent_calls: | ||
| self._settled_agent_calls.move_to_end(key) | ||
| settled_val = self._settled_agent_calls[key] | ||
| try: | ||
| loop = asyncio.get_running_loop() | ||
| except RuntimeError: | ||
| loop = asyncio.get_event_loop_policy().get_event_loop() | ||
| settled_fut: asyncio.Future[Any] = loop.create_future() | ||
| settled_fut.set_result(settled_val) | ||
| return settled_fut | ||
| in_flight = self._in_flight_agent_calls.get(key) | ||
| if in_flight is not None: | ||
| return in_flight | ||
| pending = asyncio.ensure_future(caller(call)) |
There was a problem hiding this comment.
Cache hits call get_event_loop() without a fallback loop, and cache misses pass caller(call) to asyncio.ensure_future() without a loop. In Python 3.12+, get_event_loop() raises RuntimeError on worker threads or after asyncio.run() clears the loop. Synchronous resolve_dynamic_value or subscribe_dynamic_value callers crash with RuntimeError on cache hits or coroutine agent_function_caller hooks.
| if key is not None: | |
| if key in self._settled_agent_calls: | |
| self._settled_agent_calls.move_to_end(key) | |
| settled_val = self._settled_agent_calls[key] | |
| try: | |
| loop = asyncio.get_running_loop() | |
| except RuntimeError: | |
| loop = asyncio.get_event_loop_policy().get_event_loop() | |
| settled_fut: asyncio.Future[Any] = loop.create_future() | |
| settled_fut.set_result(settled_val) | |
| return settled_fut | |
| in_flight = self._in_flight_agent_calls.get(key) | |
| if in_flight is not None: | |
| return in_flight | |
| pending = asyncio.ensure_future(caller(call)) | |
| def _get_loop() -> asyncio.AbstractEventLoop: | |
| try: | |
| return asyncio.get_running_loop() | |
| except RuntimeError: | |
| try: | |
| loop = asyncio.get_event_loop_policy().get_event_loop() | |
| if not loop.is_closed(): | |
| return loop | |
| except RuntimeError: | |
| pass | |
| loop = asyncio.new_event_loop() | |
| asyncio.set_event_loop(loop) | |
| return loop | |
| if key is not None: | |
| if key in self._settled_agent_calls: | |
| self._settled_agent_calls.move_to_end(key) | |
| settled_val = self._settled_agent_calls[key] | |
| settled_fut: asyncio.Future[Any] = _get_loop().create_future() | |
| settled_fut.set_result(settled_val) | |
| return settled_fut | |
| in_flight = self._in_flight_agent_calls.get(key) | |
| if in_flight is not None: | |
| return in_flight | |
| raw_pending = caller(call) | |
| pending = ( | |
| raw_pending | |
| if isinstance(raw_pending, asyncio.Future) | |
| else asyncio.ensure_future(raw_pending, loop=_get_loop()) | |
| ) |
🤖 Agent-generated
| local_fn = fn | ||
| local_catalog = target_catalog | ||
| return self._run_when_args_settled( | ||
| name, | ||
| self.resolve_dynamic_value( | ||
| raw_args, | ||
| peek=False, | ||
| abort_signal=abort_signal, | ||
| _keep_failed=True, | ||
| ), | ||
| lambda args: self._run_local_function( | ||
| local_fn, local_catalog, args, abort_signal | ||
| ), | ||
| abort_signal, | ||
| is_action=is_action, |
There was a problem hiding this comment.
resolve_action resets _user_activated and _is_action_execution in finally immediately upon returning a deferred _PendingResult. When pending arguments settle later, _run_local_function calls fn.execute with both flags reverted to False. Deferred local action functions observe ctx.is_user_activated == False and fail to bypass the cache on nested evaluations.
| local_fn = fn | |
| local_catalog = target_catalog | |
| return self._run_when_args_settled( | |
| name, | |
| self.resolve_dynamic_value( | |
| raw_args, | |
| peek=False, | |
| abort_signal=abort_signal, | |
| _keep_failed=True, | |
| ), | |
| lambda args: self._run_local_function( | |
| local_fn, local_catalog, args, abort_signal | |
| ), | |
| abort_signal, | |
| is_action=is_action, | |
| local_fn = fn | |
| local_catalog = target_catalog | |
| effective_in_action = is_action or self._in_action_execution | |
| def _invoke_local(args: Any) -> Any: | |
| prev_ua = self._user_activated | |
| prev_act = self._is_action_execution | |
| self._user_activated = effective_user_activated | |
| self._is_action_execution = effective_in_action | |
| try: | |
| return self._run_local_function( | |
| local_fn, local_catalog, args, abort_signal | |
| ) | |
| finally: | |
| self._user_activated = prev_ua | |
| self._is_action_execution = prev_act | |
| return self._run_when_args_settled( | |
| name, | |
| self.resolve_dynamic_value( | |
| raw_args, | |
| peek=False, | |
| abort_signal=abort_signal, | |
| _keep_failed=True, | |
| ), | |
| _invoke_local, | |
| abort_signal, | |
| is_action=is_action, |
🤖 Agent-generated
|
|
||
| result.when_settled(_on_settled) | ||
| else: | ||
| target.settle(self._peek_value(result)) |
There was a problem hiding this comment.
When a local function returns a Signal after pending arguments settle, _forward_result peeks result once without subscribing to later updates. subscribe_dynamic_value receives only the initial snapshot of the returned Signal and drops all subsequent stream updates.
Pass abort_signal to _forward_result and subscribe when result exposes .subscribe, updating target.value on subsequent emissions until aborted.
🤖 Agent-generated
| len(self._settled_agent_calls) >= _MAX_AGENT_CALL_CACHE_ENTRIES | ||
| ): | ||
| self._settled_agent_calls.popitem(last=False) | ||
| self._settled_agent_calls[key] = done.result() |
There was a problem hiding this comment.
_settled_agent_calls stores done.result() directly and returns self._settled_agent_calls[key] on cache hits without copying. Unlike dispatch_error, callers share the cached dict or list instance. In-place mutation of a resolved dict or list by one caller corrupts subsequent reactive cache hits on the surface.
Wrap cached values with copy.deepcopy both when storing into _settled_agent_calls (line 246) and when reading cache hits (line 218).
🤖 Agent-generated
| or silently evaluating to `None` (schema-only). This applies to bindings, | ||
| check rules and `action.functionCall`. The value is pending (`None`, or a | ||
| pending signal from `resolve_dynamic_value(..., peek=False)` and | ||
| `subscribe_dynamic_value`) until the `agentFunctionResponse` arrives; an |
There was a problem hiding this comment.
The changelog omits action.event (context and userMessage), which also awaits pending agent calls. It also states subscribe_dynamic_value returns a pending signal, whereas it returns a Subscription with value == None. Consumers reading the release notes will expect a Signal from subscribe_dynamic_value and miss async action.event support.
| or silently evaluating to `None` (schema-only). This applies to bindings, | |
| check rules and `action.functionCall`. The value is pending (`None`, or a | |
| pending signal from `resolve_dynamic_value(..., peek=False)` and | |
| `subscribe_dynamic_value`) until the `agentFunctionResponse` arrives; an | |
| or silently evaluating to `None` (schema-only). This applies to bindings, | |
| check rules, `action.functionCall`, and `action.event` (`context` and | |
| `userMessage`). The value is pending (`None` from `resolve_dynamic_value` and | |
| `subscribe_dynamic_value`, or a pending signal from | |
| `resolve_dynamic_value(..., peek=False)`) until the `agentFunctionResponse` |
🤖 Agent-generated
| # implementation in the catalog it resolves to (undeclared, or | ||
| # declared schema-only) is routed here instead of failing, as the v1.0 | ||
| # fallback routing defines. | ||
| self.agent_function_caller: AgentFunctionCaller | None = None |
There was a problem hiding this comment.
SurfaceModel.agent_function_caller is a mutable public attribute, yet reassigning it leaves _in_flight_agent_calls and _settled_agent_calls populated and _release skips checking self.agent_function_caller is caller. Replacing a surface's caller serves stale cached answers from the previous caller instead of invoking the new caller.
Convert agent_function_caller into a property whose setter clears _in_flight_agent_calls and _settled_agent_calls when changed, and check self.agent_function_caller is caller in _release.
🤖 Agent-generated
| [#3415](https://github.com/a2ui-project/a2ui/issues/3415), | ||
| [#3536](https://github.com/a2ui-project/a2ui/issues/3536) |
There was a problem hiding this comment.
The changelog entry links issues #3415 and #3536 without a link to pull request #3740, unlike surrounding entries (such as line 45). Readers inspecting release notes cannot jump directly from the changelog entry to the pull request diff.
| [#3415](https://github.com/a2ui-project/a2ui/issues/3415), | |
| [#3536](https://github.com/a2ui-project/a2ui/issues/3536) | |
| [#3415](https://github.com/a2ui-project/a2ui/issues/3415), | |
| [#3536](https://github.com/a2ui-project/a2ui/issues/3536), | |
| [#3740](https://github.com/a2ui-project/a2ui/pull/3740) |
🤖 Agent-generated
Stacked on the Dart PR (#3739). Review only the top commit (
fix(py-core): ...).Part of #3536.
Description
The Python renderer core did not route v1.0 function calls it cannot run to the agent.
DataContextrejected an undeclared function ("Function '...' not found in catalog"), and a declared function without an implementation evaluated toNone. Only app code callingMessageProcessor.call_agent_functioncould use agent-side functions. This PR ports the routing from #3735.Changes
SurfaceModel.agent_function_callerandSurfaceModel.call_agent_function, with theAgentFunctionCallandAgentFunctionCallertypes exported from the package. Identical calls (same catalog, function and arguments) that are still awaiting a response share one request.MessageProcessorinstalls the caller on v1.0 surfaces when the processor has an outbound listener. Without one, nothing changes.DataContextroutes a call whose catalog resolved but has no implementation of the function (undeclared, or declared schema-only):catalogId.requires_user_activation, is validated against its schema, and sends the parsed arguments with defaults applied.Noneuntil the response arrives. Agent errors and timeouts are reported asEXPRESSION_ERROR.CATALOG_ERROR, pre-1.0 surfaces and@system functions are not routed.DataContexthad no awaitable handling, so a routed call returns a signal whose value isNoneuntil it settles. Existing subscriptions work unchanged. A call with a pending nested argument waits for it, so an agent result can feed a local function.DataContext.is_user_activatedis new: it is true while an action's function call is evaluated. A routed schema-only function that requires user activation is sent from actions only; bindings and checks reportEXPRESSION_ERROR. This is stricter than web_core outside a browser.allow_unknown_elementscomment now explains what renderers that use agent-side functions need to do.Testing
tests/test_agent_function_routing.py(23 tests) covers every case in the web_core suite, plus nested agent calls, a failed nested argument, user activation, stale evaluations,@system functions and JSON-schema defaults.uv run pytest python/a2ui_core/tests: 1577 passed, 15 skipped. The a2ui_agent, a2ui_a2a and a2ui_adk tests pass (1732 passed, 2 xfailed).mypyandpyink --checkare clean.