feat(spring-ai): pass ADK context through tool callbacks - #1472
feat(spring-ai): pass ADK context through tool callbacks#1472mumu-1029521 wants to merge 1 commit 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. |
b13d3cc to
a35726e
Compare
|
Hi @mumu-1029521, thank you for your contribution! We appreciate you taking the time to submit this pull request. Currently this PR is under review by our team we will keep you posted if any additional information is required. thank you. |
a35726e to
aa74564
Compare
| // Spring AI still requires callbacks to expose tool definitions to the model. The | ||
| // callback intentionally has no side effect: ADK will execute the returned function call | ||
| // later with the InvocationContext-backed ToolContext. | ||
| Function<Map<String, Object>, String> definitionOnlyCallback = arguments -> ""; |
There was a problem hiding this comment.
These need to keep executing. Your comment on line 158 is right that Spring AI gives no definition-only option and that ADK runs the call later — but on 2.0.x ChatModel.call never invokes callbacks at all, so nothing can double-run through ADK. The empty string only ever reaches callers outside ADK's flow, and it breaks them: a ChatClient does execute these callbacks, so every tool result silently becomes an empty string. For a tool that doesn't take a toolContext parameter this callback works today and returns the real result.
Spring AI's ToolCallback javadoc also says call should "return the result to send back to the AI model".
| ToolContext adkToolContext = | ||
| Objects.requireNonNull( | ||
| toolContextResolver.resolve( | ||
| tool, processedArguments, springAiToolContext), |
There was a problem hiding this comment.
Could we use the springAiToolContext you already receive here, instead of a resolver? ToolCallingChatOptions.toolContext(...) fills it and DefaultToolCallingManager passes it to call(input, toolContext). Agree a key and both branches collapse into one callback:
public static final String ADK_TOOL_CONTEXT = "adk_tool_context";
BiFunction<Map<String, Object>, org.springframework.ai.chat.model.ToolContext, Map<String, Object>>
callback =
(arguments, springAiToolContext) -> {
Map<String, Object> processed = processArguments(arguments, declaration);
return tool.runAsync(processed, adkToolContextFrom(springAiToolContext)).blockingGet();
};with adkToolContextFrom reading the key and throwing a clear error if the value isn't an ADK ToolContext.
To be straight about the limits: this gives callers a supported way to pass ADK context in, and a caller who passes nothing keeps today's behaviour. It doesn't by itself close #699 — that reporter passes an empty map and calls functionCallId(), and Spring AI shares one context across a whole round-trip, so it can't carry a per-call id. Closing #699 properly needs ADK to derive the context per call; this is the channel to carry it. One visible change: a throwing tool would surface as ToolExecutionException rather than a wrapped RuntimeException.
On the name, adk_tool_context matches adk_request_confirmation in core.
| * callback is invoked directly without a context | ||
| * @return the ADK context to pass to the tool; must not be {@code null} | ||
| */ | ||
| ToolContext resolve( |
There was a problem hiding this comment.
With the context travelling in Spring AI's map, I don't think this earns a public type — a resolver that just reads a key is a lambda the caller can keep locally, and every public type in contrib/ is one more to keep compatible. It is a real trade though: you'd lose per-call dispatch, the per-call functionCallId, and the requireNonNull fail-fast. If per-call context is something you need, say so and it should stay.
Either way, SpringAIAutoConfiguration calls getIfAvailable() on this bean at lines 105, 132 and 159, so an app with two of them fails to start with "expected single matching bean but found 2" — even in the default mode, where the resolver is never used. getIfUnique() returns null for both zero and many, which lands on the check you already have.
| package com.google.adk.models.springai; | ||
|
|
||
| /** Selects which framework owns the tool-calling lifecycle. */ | ||
| public enum ToolExecutionMode { |
There was a problem hiding this comment.
I don't think we need this mode. When Spring AI executes an ADK tool the call never becomes an ADK Event, so the session has no record of it. Live Gemini, one plain tool, no sub-agents:
ADK_MANAGED 4 events: user · functionCall · functionResponse · answer
SPRING_AI_MANAGED 2 events: user · answer
Both answered correctly; only one recorded that a tool ran, so replay, resumption, memory and evals see a model answering from nowhere. transfer_to_agent and ExitLoopTool break the same way — they work only by mutating ToolContext actions, so the call reports success, the specialist never runs, and no transfer is recorded. No exception, no log.
With the change in the previous comment there's nothing left for the mode to switch between: ADK's flow never invokes the callback, and a caller who passes a context gets a real one. If there's a case that still needs it, I'd rather see it than guess.
Minor: ToolExecutionMode is already a public enum in core/.../agents/RunConfig.java, where it means how concurrently rather than who.
| callbackBuilder.inputSchema(schemaJson); | ||
| logger.debug("Set input schema JSON for {}: {}", toolName, schemaJson); | ||
| } catch (JsonProcessingException e) { | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
This used to log and carry on; now one tool with an unserialisable schema fails every request, since convertToSpringAiTools runs on each one. Throwing is probably the better behaviour, but it's a behaviour change that isn't in the description and nothing tests it.
|
Thanks for this. Your diagnosis is right — the One thing worth knowing, and it isn't anything you did. On the version this module pins, #699 no longer reproduces through ADK — Spring AI 2.0 moved the tool-calling loop out of If you'd rather not carry another round, say so and we'll take it from here with credit to you. If you decide to apply the fixes, could you also squash the commits into 1 as per our policy? Thanks in advance. |
78039b3 to
a6f1776
Compare
|
Thanks, @kvmilos. I've applied the requested simplification.
I also updated the title and description to use feat and to make the limitation explicit: this provides a context propagation channel, but it does not derive a per-call ADK context or functionCallId, so it does not fully resolve the behavior reported in #699 by itself. Local verification: Could you please take another look? |
|
Thanks, @kvmilos. I've applied the requested simplification.
- Removed `ToolExecutionMode` and `AdkToolContextResolver`.
- Removed the definition-only callback and kept converted callbacks executable.
- Collapsed both branches into one `FunctionToolCallback` `BiFunction`.
- Added `ToolConverter.ADK_TOOL_CONTEXT` with the `adk_tool_context` key.
- The callback now passes an ADK `ToolContext` from Spring AI's context map when
present, preserves the existing `null` behavior when absent, and reports a
clear error for values of the wrong type.
- Restored the previous log-and-continue schema serialization behavior.
- Removed the debug block that logged raw tool arguments.
- Updated the tests for direct invocation, empty context, context propagation,
and invalid context types.
- Rebased onto the current `main` and squashed the PR to one commit.
I also updated the title and description to use `feat` and to make the limitation
explicit: this provides a context propagation channel, but it does not derive a
per-call ADK context or `functionCallId`, so it does not by itself fully close
#699.
Local verification:
```text
./mvnw -pl contrib/spring-ai clean test
Tests run: 213, Failures: 0, Errors: 0, Skipped: 20
BUILD SUCCESS
At 2026-09-11 00:38:30, "Kamilos" ***@***.***> wrote:
kvmilos left a comment (google/adk-java#1472)
Thanks for this.
Your diagnosis is right — the SPRING_AI_MANAGED branch already takes FunctionToolCallback's BiFunction overload, so it already receives Spring AI's own ToolContext. What I'd like to ask for is smaller than what you've written: it deletes two of the new public types rather than adding anything. Dropping the debug block that logged raw tool arguments was worth doing on its own.
One thing worth knowing, and it isn't anything you did. On the version this module pins, #699 no longer reproduces through ADK — Spring AI 2.0 moved the tool-calling loop out of ChatModel, so ADK's own flow runs tools with a real ToolContext. That arrived with the June upgrade, months after #699 was filed; nothing in the issue or the changelog says so, and none of us noticed either. What's still broken is the path #699's reporter actually used: calling ToolCallback.call(...) directly, or handing ToolConverter's output to a ChatClient.
Both red checks clear on one rebase — git fetch upstream && git rebase upstream/main, then git push --force-with-lease; don't pass --rebase-merges.
Separately: the title should be feat, not fix. It becomes the squash commit release-please reads, and this adds public API.
If you'd rather not carry another round after this long, say so and we'll take it from here with credit to you.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
ToolConverter creates Spring AI ToolCallback instances that execute ADK tools. When a callback is invoked directly, or when the converted callback is passed to a Spring AI ChatClient, the existing implementation always calls:
This prevents callers that already have an ADK ToolContext from passing it to the tool.
On the Spring AI version currently used by this module, this problem no longer reproduces through ADK's normal model flow: Spring AI 2.0 moved the tool-calling loop out of ChatModel, so ADK executes those tool calls with its own invocation-derived context. The remaining affected paths are direct ToolCallback.call(...) calls and consumers such as ChatClient that execute the converted callbacks themselves.
Solution:
Example:
This change provides a supported channel for callers to pass an existing ADK ToolContext. It does not derive an ADK context for each individual Spring AI tool call and therefore does not by itself fully resolve the behavior reported in #699. In particular, Spring AI shares one tool context across a round trip, so this change cannot synthesize a per-call functionCallId for a caller that supplies no ADK context.
One visible compatibility change is that exceptions thrown during tool execution now surface through Spring AI's standard ToolExecutionException instead of an additional wrapped RuntimeException.
Testing Plan
Unit Tests:
Coverage includes:
Direct callback invocation without a Spring AI context executes the tool and preserves the existing null ADK context behavior.
An empty Spring AI context map preserves the existing behavior.
An ADK ToolContext stored under ADK_TOOL_CONTEXT reaches the tool unchanged.
A value of the wrong type produces a clear ToolExecutionException rooted in an IllegalArgumentException.
A wrong context value does not invoke the tool.
Existing schema conversion and provider-specific argument normalization tests continue to pass.
./mvnw -pl contrib/spring-ai clean test
Tests run: 213, Failures: 0, Errors: 0, Skipped: 20
BUILD SUCCESS
The module and its reactor dependencies also pass:
Manual End-to-End (E2E) Tests:
A live external-provider E2E test was not run because it requires provider credentials. The direct callback behavior and Spring AI context propagation are covered by deterministic local tests.
Checklist
Additional context
This revision intentionally removes the previously proposed ToolExecutionMode and AdkToolContextResolver public types. It also removes the ADK-managed/Spring-AI-managed execution-mode split. The converted callback has one behavior: execute the tool, optionally using the ADK ToolContext supplied through Spring AI's context map.