refactor: extract shared tool execution support by arnabnandy7 · Pull Request #1837 · embabel/embabel-agent

arnabnandy7 · 2026-07-28T16:43:11Z

Summary

Extracts provider-independent tool execution and dynamic tool injection behavior from DefaultToolLoop into reusable support functions.

This creates a shared foundation for other tool-loop implementations, including future streaming tool loops, without introducing streaming-specific behavior in this PR.

Changes

Extract tool invocation and result-content conversion.
Preserve before and after tool-call inspector notifications.
Extract ToolInjectionStrategy evaluation.
Preserve structured JSON result deserialization.
Preserve tool decoration, deduplication, addition, removal, and tracking.
Update DefaultToolLoop to use the shared implementation.

Motivation

Tool execution behavior was embedded in DefaultToolLoop, making it difficult for other tool-loop implementations to reuse the same semantics. Centralizing this behavior avoids duplication and helps keep blocking, parallel, and future streaming implementations consistent.

Testing

mvn -pl embabel-agent-api -Dtest=ToolLoopTest,ParallelToolLoopTest test

Result: 24 tests passed with no failures or errors.

Closes #1836

igordayen · 2026-07-28T18:54:26Z

@arnabnandy7 - will review and retest IT tests, thanks

arnabnandy7 · 2026-07-28T19:09:05Z

@arnabnandy7 - will review and retest IT tests, thanks

this PR should be merged before #1826, so with this that PR has two dependencies which should be resolved first before we act on it.

igordayen

@arnabnandy7 - thanks, left few comments for consideration / as inquiries. regards

igordayen · 2026-07-30T00:09:37Z

What is your vision of support if the number of tool loop implementations grows? Inheritance vs composition .... thanks

And please mark as "resolved".

arnabnandy7 · 2026-07-30T13:09:08Z

What is your vision of support if the number of tool loop implementations grows? Inheritance vs composition .... thanks

My preference is composition for the shared tool-execution behavior. Each tool loop should own its orchestration model sequential, parallel, or others etc while reusing the common execution, callback, result-conversion, and injection logic.

ParallelToolLoop currently extends DefaultToolLoop because it’s a variation of the same loop and shares most of its lifecycle. However, I wouldn’t keep growing that inheritance hierarchy for unrelated implementations. The stateless helpers introduced in this PR are a step toward composition, allowing future loops to reuse the same behavior without extending DefaultToolLoop.

If this shared logic eventually needs more dependencies or state, we can turn it into an injected support component instead of adding more responsibilities to the base class.

And please mark as "resolved".

Done, all marked resolved.

igordayen · 2026-07-30T13:39:53Z

@arnabnandy7 testing PR, will update on completion of testing

igordayen · 2026-07-30T13:55:37Z

tool-loop-IT.txt

testing from embabel-agent-experimental repo.

igordayen · 2026-07-30T14:01:18Z

Found issue:

Caused by: java.util.concurrent.ExecutionException: java.lang.NoSuchMethodError: 'tools.jackson.databind.node.ObjectNode com.github.victools.jsonschema.generator.SchemaGenerator.generateSchema(java.lang.reflect.Type, java.lang.reflect.Type[])'
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:396)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2096)
	at com.embabel.agent.spi.support.AbstractLlmOperations.executeWithTimeout(AbstractLlmOperations.kt:108)
	... 12 more
Caused by: java.lang.NoSuchMethodError: 'tools.jackson.databind.node.ObjectNode com.github.victools.jsonschema.generator.SchemaGenerator.generateSchema(java.lang.reflect.Type, java.lang.reflect.Type[])'
	at com.embabel.common.ai.converters.JacksonOutputConverter.jsonSchemaValue_delegate$lambda$0(JacksonOutputConverter.kt:104)
	at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:86)
	at com.embabel.common.ai.converters.JacksonOutputConverter.getJsonSchemaValue(JacksonOutputConverter.kt:101)
	at com.embabel.common.ai.converters.JacksonOutputConverter.getJsonSchema(JacksonOutputConverter.kt:190)
	at com.embabel.common.ai.converters.JacksonOutputConverter.getFormat(JacksonOutputConverter.kt:199)
	at com.embabel.agent.spi.support.springai.SuppressThinkingConverter.getFormat(SuppressThinkingConverter.kt:95)
	at com.embabel.agent.spi.support.springai.WithExampleConverter.getFormat(WithExampleConverter.kt:73)
	at com.embabel.agent.spi.support.springai.ExceptionWrappingConverter.getFormat(ExceptionWrappingConverter.kt:45)
	at com.embabel.agent.spi.support.springai.SpringAiOutputConverterAdapter.getFormat(ChatClientLlmOperations.kt:971)
	at com.embabel.agent.spi.support.ToolLoopLlmOperations.doTransformWithThinking(ToolLoopLlmOperations.kt:356)
	at com.embabel.agent.spi.support.AbstractLlmOperations.createObjectWithThinking$lambda$1$0$0(AbstractLlmOperations.kt:341)
	at com.embabel.agent.spi.support.ExecutorAsyncer.async$lambda$0(ExecutorAsyncer.kt:53)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1768)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)

requires upgrading

igordayen · 2026-07-30T14:04:31Z

Conclusion:

IT tests in embabel-agent passed
Tests in the experimental repo failed; need to upgrade it.

thanks

arnabnandy7 · 2026-07-30T14:04:40Z

requires upgrading

Thanks for catching this. The failure happens during output-schema generation, before either tool-loop implementation executes, and both the default and parallel ITs fail with the same NoSuchMethodError.

It looks like the experimental repository is resolving an incompatible VicTools jsonschema-generator version. The current agent branch resolves jsonschema-generator:5.0.0, which matches the Jackson 3 return type expected by embabel-common.

I’ll align/upgrade the experimental repository’s Embabel BOM or dependency version and rerun the IT suite. This appears to be a runtime dependency alignment issue rather than a regression in the shared tool-execution change.

arnabnandy7 · 2026-07-30T14:06:50Z

@igordayen on another note, can you try running with -U and let me know what happening before I try upgrading anything in this PR??

mvn -U -Dtest='*IT,!LLMOllama*IT' \
  -Dsurefire.failIfNoSpecifiedTests=false \
  clean test

I checked the dependency graph. This branch resolves jsonschema-generator:5.0.0, which has the Jackson 3 signature expected by embabel-common.

The experimental repository uses the mutable 1.5.0-SNAPSHOT parent, and the test command ran without -U, so this looks like an older snapshot dependency being reused from the local Maven cache.

There doesn’t appear to be a code change required in this PR.

igordayen · 2026-07-30T14:24:41Z

@igordayen on another note, can you try running with -U and let me know what happening before I try upgrading anything in this PR??
mvn -U -Dtest='*IT,!LLMOllama*IT' \
  -Dsurefire.failIfNoSpecifiedTests=false \
  clean test

I will later in a day, a bit busy, outside, thanks for patience.

igordayen · 2026-07-30T15:17:57Z

gordayen@pop-os:~/git/embabel-agent-experimental/embabel-experimental-integration-tests$ mvn dependency:tree|grep jackson
[INFO] +- com.fasterxml.jackson.module:jackson-module-kotlin:jar:2.21.4:compile (optional)
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.21.4:compile
[INFO] |  |  \- com.fasterxml.jackson.core:jackson-core:jar:2.21.4:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.21:compile
[INFO] |  |  |  +- com.fasterxml.jackson.datatype:jackson-datatype-jdk8:jar:2.21.4:compile
[INFO] |  |  |  +- com.fasterxml.jackson.datatype:jackson-datatype-jsr310:jar:2.21.4:compile
[INFO] |  |  |  +- com.github.victools:jsonschema-module-jackson:jar:4.38.0:compile
[INFO] |  |  |  \- tools.jackson.dataformat:jackson-dataformat-yaml:jar:3.1.4:compile
[INFO] |  |  |     +- tools.jackson.core:jackson-core:jar:3.1.4:compile
[INFO] |  |  |     +- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.21.4:compile
[INFO] |  |  |  |     +- io.modelcontextprotocol.sdk:mcp-json-jackson3:jar:2.0.0:compile
[INFO] |  |  +- tools.jackson.module:jackson-module-kotlin:jar:3.1.4:compile
[INFO] |  |  |  \- tools.jackson.core:jackson-databind:jar:3.1.4:compile
igordayen@pop-os:~/git/embabel-agent-experimental/embabel-experimental-integration-tests$

igordayen · 2026-07-30T15:20:24Z

mvn -U -Dtest='IT,!LLMOllamaIT'
-Dsurefire.failIfNoSpecifiedTests=false
clean test ==> same errors

igordayen · 2026-07-30T15:23:06Z

found:
/git/embabel-agent-experimental/embabel-experimental-integration-tests$ grep -r jackson . --include "pom.xml"
./pom.xml: com.fasterxml.jackson.module
./pom.xml: jackson-module-kotlin

grep -r jackson . --include "*.java"
./src/main/java/com/embabel/agent/api/tool/loop/testing/AbstractToolLoopTest.java:import com.fasterxml.jackson.annotation.JsonClassDescription;
./src/main/java/com/embabel/agent/api/tool/loop/testing/AbstractToolLoopTest.java:import com.fasterxml.jackson.annotation.JsonPropertyDescription;
./src/test/java/com/embabel/agent/api/tool/loop/OpenAiToolLoopIT.java:import tools.jackson.databind.json.JsonMapper;
./src/test/java/com/embabel/agent/api/tool/loop/LangChainToolLoopIT.java:import tools.jackson.databind.json.JsonMapper;

arnabnandy7 · 2026-07-30T16:44:48Z

@igordayen thanks, this confirms the mismatch is in the experimental integration-test module. It still explicitly depends on the Jackson 2 Kotlin module and resolves jsonschema-module-jackson:4.38.0.

The Kotlin module dependency should be migrated to tools.jackson.module:jackson-module-kotlin, and VicTools should resolve consistently to 5.0.0. The com.fasterxml.jackson.annotation imports can remain as they are because Jackson 3 still uses the Jackson 2 annotations package.

This should be fixed in embabel-agent-experimental, rather than in this PR. After updating it, we can verify with:

mvn dependency:tree -Dincludes=com.github.victools:*,tools.jackson*:*,com.fasterxml.jackson*:*

Both jsonschema-generator and jsonschema-module-jackson should resolve to 5.0.0 before rerunning the IT suite.

igordayen · 2026-07-30T17:45:17Z

embabel/embabel-agent-experimental#27 - refactoring passed after upgrade, thanks

igordayen · 2026-07-31T01:11:53Z

@arnabnandy7 - I'm ready to merge, as PR "Replace Spring AI deprecated API " got merged.

Please confirm, thanks

arnabnandy7 · 2026-07-31T03:29:54Z

@arnabnandy7 - I'm ready to merge, as PR "Replace Spring AI deprecated API " got merged.

Please confirm, thanks

I'll prepare and let you know. Thanks 😊

arnabnandy7 · 2026-07-31T05:36:50Z

Please confirm, thanks

I've synced this PR with main, please proceed. @igordayen

Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>

arnabnandy7 · 2026-07-31T05:39:25Z

I've squashed the commits as well.

igordayen

@arnabnandy7 - all integration tests successfully passed, thank you

arnabnandy7 mentioned this pull request Jul 28, 2026

feat: add vendor-neutral streaming tool loop #1826

Merged

igordayen reviewed Jul 28, 2026

View reviewed changes

arnabnandy7 requested a review from igordayen July 29, 2026 16:17

igordayen mentioned this pull request Jul 30, 2026

Replace Spring AI 2.0 deprecated API #1840

Merged

arnabnandy7 force-pushed the chore/shared-tool-execution-support branch from 601a8a1 to 844fe2b Compare July 31, 2026 05:34

refactor: extract shared tool execution support

30e407b

Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>

arnabnandy7 force-pushed the chore/shared-tool-execution-support branch from 844fe2b to 30e407b Compare July 31, 2026 05:38

igordayen approved these changes Jul 31, 2026

View reviewed changes

igordayen merged commit 0273e76 into embabel:main Jul 31, 2026
1 check passed

Uh oh!

Conversation

arnabnandy7 commented Jul 28, 2026

Summary

Changes

Motivation

Testing

Uh oh!

igordayen commented Jul 28, 2026

Uh oh!

arnabnandy7 commented Jul 28, 2026

Uh oh!

igordayen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

arnabnandy7 commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

arnabnandy7 commented Jul 30, 2026

Uh oh!

arnabnandy7 commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arnabnandy7 commented Jul 30, 2026

Uh oh!

igordayen commented Jul 30, 2026

Uh oh!

igordayen commented Jul 31, 2026

Uh oh!

arnabnandy7 commented Jul 31, 2026

Uh oh!

arnabnandy7 commented Jul 31, 2026

Uh oh!

arnabnandy7 commented Jul 31, 2026

Uh oh!

igordayen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arnabnandy7 commented Jul 30, 2026 •

edited

Loading

igordayen commented Jul 30, 2026 •

edited

Loading