feat(langchain): Add comprehensive AI Agent and Tooling support #389

ginccc · 2025-12-02T18:04:11Z

Implemented Declarative Agents framework (DeclarativeAgent, DeclarativeAgentTask)
Added infrastructure for tool execution: ToolExecutionService, caching, cost tracking, and rate limiting
Implemented EddiToolBridge for integrating EDDI HTTP calls and EddiChatMemoryStore for memory integration
Added extensive suite of built-in tools: Calculator, DataFormatter, DateTime, PdfReader, TextSummarizer, Weather, WebScraper, WebSearch
Added comprehensive documentation for Bot Father

This change is

- Implemented Declarative Agents framework (`DeclarativeAgent`, `DeclarativeAgentTask`) - Added infrastructure for tool execution: `ToolExecutionService`, caching, cost tracking, and rate limiting - Implemented `EddiToolBridge` for integrating EDDI HTTP calls and `EddiChatMemoryStore` for memory integration - Added extensive suite of built-in tools: Calculator, DataFormatter, DateTime, PdfReader, TextSummarizer, Weather, WebScraper, WebSearch - Added comprehensive documentation for Bot Father

Copilot

Pull request overview

This pull request introduces comprehensive AI Agent and Tooling support for EDDI's LangChain integration. The implementation adds a declarative agents framework, tool execution infrastructure with caching/cost tracking/rate limiting, and a suite of built-in tools (Calculator, DataFormatter, DateTime, PdfReader, TextSummarizer, Weather, WebScraper, WebSearch). The PR also includes extensive documentation updates for Bot Father and architectural improvements to lifecycle management.

Key Changes

Implemented declarative agents framework with DeclarativeAgent and tool execution services
Added 8 built-in tools with smart caching (tool-specific TTL values), cost tracking, and rate limiting capabilities
Integrated EDDI HTTP calls and conversation memory with LangChain4j through bridge components
Extracted HttpCallExecutor for reusability across lifecycle tasks and agent tools
Enhanced documentation with comprehensive guides for agent orchestration, pattern matching, and lifecycle management

Reviewed changes

Copilot reviewed 151 out of 178 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`ToolCacheService.java`	Implements Infinispan-based caching with tool-specific TTL values and metrics
`EddiToolBridge.java`	Bridges LangChain agents with EDDI HTTP calls for secure tool execution
`RestToolHistory.java`	REST API for tool execution history, cache stats, rate limits, and cost tracking
`ToolExecutionTrace.java`	Tracks tool calls with metrics, caching info, and cost data for analytics
`LangChainConfiguration.java`	Unified configuration supporting legacy chat and declarative agent modes
`CustomToolConfiguration.java`	Allows JSON-based custom tool definitions without Java code
`EddiChatMemoryStore.java`	Bridges EDDI conversation memory with LangChain4j chat memory
`LangchainTask.java`	Enhanced to support both simple chat and agent mode with tool execution
`AgentExecutionHelper.java`	Manages agent tool execution with retry logic and tool collection
`DeclarativeAgent.java`	AI Service interface for declarative agents with automatic tool calling
`IHttpCallExecutor.java`	Interface abstraction for HTTP call execution across tasks
`HttpCallsTask.java`	Refactored to use `IHttpCallExecutor` for shared execution logic
`HttpCallExecutor.java`	Extracted HTTP call execution logic for reusability in agent tools
`ConversationCoordinator.java`	Enhanced with comprehensive documentation explaining conversation ordering
`LifecycleManager.java`	Enhanced with detailed lifecycle pipeline documentation
`ILifecycleTask.java`	Added extensive interface documentation explaining lifecycle architecture
`HttpCall.java`	Added `description` and `parameters` fields for LLM agent tool integration
`Dockerfile.jvm`	Updated base image from 1.22 to 1.23
`pom.xml`	Version bump to 5.6.0, updated dependencies for LangChain4j, Quarkus, and added tool libraries
`mvnw.cmd`	Updated Maven wrapper from 3.2.0 to 3.3.4
`mvnw`	Updated Maven wrapper from 3.2.0 to 3.3.4
Documentation files	Extensive updates to semantic-parser, langchain, metrics, and other guides

Comments suppressed due to low confidence (1)

pom.xml:1

Version 1.23 of openjdk-21-runtime may not exist or be valid. Verify this version is available in the Red Hat registry. As of the knowledge cutoff (January 2025), the latest available version should be confirmed before deployment.

<?xml version="1.0"?>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T18:05:59Z

src/main/java/ai/labs/eddi/modules/langchain/tools/ToolCacheService.java

+     */
+    private String buildKey(String toolName, String arguments) {
+        // Use tool name + hash of arguments for key
+        return toolName + ":" + Math.abs(arguments.hashCode());


Hash collision risk: using hashCode() alone can cause cache key collisions for different arguments with the same hash. Consider using a cryptographic hash (e.g., SHA-256) or at minimum include a delimiter and the full arguments string to ensure uniqueness.

Copilot · 2025-12-02T18:05:59Z

src/main/java/ai/labs/eddi/modules/langchain/tools/EddiToolBridge.java

+            // Execute the httpcall using the shared executor
+            Map<String, Object> result = httpCallExecutor.execute(
+                    httpCall,
+                    null, // Memory would be injected properly in DeclarativeAgentTask context
+                    templateData,
+                    config.getTargetServerUrl()
+            );
+
+            // Return the result as JSON for the agent to process
+            return jsonSerialization.serialize(result);
+


Passing null for memory can cause NullPointerException in httpCallExecutor.execute() when it tries to access memory.getCurrentStep() at line 68 of HttpCallExecutor.java. Either inject actual memory or add null checks in the executor.

Suggested change

// Execute the httpcall using the shared executor

Map<String, Object> result = httpCallExecutor.execute(

httpCall,

null, // Memory would be injected properly in DeclarativeAgentTask context

templateData,

config.getTargetServerUrl()

);

// Return the result as JSON for the agent to process

return jsonSerialization.serialize(result);

// Prevent NullPointerException: do not call executor with null memory

return errorResult("Cannot execute httpcall: conversation memory is required but not available in this context.");

Copilot · 2025-12-02T18:06:00Z

src/main/java/ai/labs/eddi/modules/langchain/rest/RestToolHistory.java

+    ToolCostTracker costTracker;
+
+    // In-memory storage for conversation traces (in production, use database)
+    private final Map<String, ToolExecutionTrace> conversationTraces = new HashMap<>();


In-memory storage for conversation traces will be lost on service restart and doesn't scale across multiple instances. The comment acknowledges this ('in production, use database'), but this creates a production-readiness gap. Consider implementing persistent storage or documenting this limitation more prominently.

Copilot · 2025-12-02T18:06:00Z

src/main/java/ai/labs/eddi/modules/langchain/impl/LangchainTask.java

+
+        if (!isNullOrEmpty(processedParams.get(KEY_CONVERT_TO_OBJECT)) &&
+                Boolean.parseBoolean(processedParams.get(KEY_CONVERT_TO_OBJECT))) {
+            var contentAsObject = jsonSerialization.deserialize(responseContent, Map.class);


The parameter reference is incorrect: should deserialize responseContent not processedParams.get(KEY_CONVERT_TO_OBJECT). The original code on line 287 was checking the parameter but line 288 references the wrong variable.

Copilot · 2025-12-02T18:06:00Z

src/main/java/ai/labs/eddi/modules/langchain/impl/AgentExecutionHelper.java

+    private static boolean isRetryableError(Exception e) {
+        String message = e.getMessage() != null ? e.getMessage().toLowerCase() : "";


Error detection based on string matching in exception messages is fragile and locale-dependent. Consider checking exception types (e.g., SocketTimeoutException, HttpStatusException) or using structured error codes for more reliable retry logic.

Copilot · 2025-12-02T18:06:01Z

src/main/java/ai/labs/eddi/modules/httpcalls/impl/HttpCallExecutor.java

+    public Map<String, Object> execute(HttpCall call,
+                                        IConversationMemory memory,
+                                        Map<String, Object> templateDataObjects,
+                                        String targetServerUrl) throws LifecycleException {


Potential NullPointerException: memory parameter can be null (as evidenced by EddiToolBridge.java:125 passing null). Add null check: if (memory == null) throw new IllegalArgumentException(\"memory cannot be null\");

Suggested change

String targetServerUrl) throws LifecycleException {

String targetServerUrl) throws LifecycleException {

if (memory == null) {

throw new IllegalArgumentException("memory cannot be null");

}

…ather tools

feat(langchain): Enhance error handling for retryable errors in AgentExecutionHelper feat(langchain): Refactor tool history retrieval in RestToolHistory to use conversation memory store feat(langchain): Create temporary memory instance in EddiToolBridge for HttpCallExecutor feat(langchain): Improve cache key generation in ToolCacheService for long arguments

…shot and ResultSnapshot

…pCallExecutor execution

Copilot

Pull request overview

Copilot reviewed 151 out of 179 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T20:33:09Z

src/main/java/ai/labs/eddi/modules/langchain/tools/EddiToolBridge.java

+    ToolExecutionService toolExecutionService;
+
+    // Cache for httpcall configurations to avoid repeated lookups
+    private final Map<String, HttpCallsConfiguration> configCache = new HashMap<>();


The configCache HashMap is not thread-safe and could lead to race conditions in concurrent access scenarios. Since this is an ApplicationScoped CDI bean, multiple threads may access it simultaneously. Use ConcurrentHashMap instead of HashMap.

Copilot · 2025-12-02T20:33:10Z

src/main/java/ai/labs/eddi/modules/langchain/impl/LangchainTask.java

 import static ai.labs.eddi.configs.packages.model.ExtensionDescriptor.FieldType;
 import static ai.labs.eddi.utils.RuntimeUtilities.isNullOrEmpty;
-import static java.util.Objects.requireNonNullElse;



Removed import 'requireNonNullElse' but line 275-276 still appear to use similar null handling logic. Verify that null handling is still correct without this import, or if alternative null-safe handling was implemented elsewhere.

…all configuration caching

Copilot

Pull request overview

Copilot reviewed 151 out of 179 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T21:44:58Z

src/main/java/ai/labs/eddi/modules/langchain/tools/EddiToolBridge.java

+            // Build template data by merging agent arguments with conversation memory
+            // Note: In real usage, we'd need an IConversationMemory instance, not a snapshot
+            // This is a simplified version - the DeclarativeAgentTask will handle this properly
+            Map<String, Object> templateData = new HashMap<>(arguments);


Missing import statement for HashMap. Add import java.util.HashMap; to resolve compilation error.

Copilot · 2025-12-02T21:44:58Z

src/main/java/ai/labs/eddi/modules/langchain/tools/EddiToolBridge.java

+     * Creates a JSON error result for the agent
+     */
+    private String errorResult(String errorMessage) {
+        Map<String, Object> error = new HashMap<>();


Missing import statement for HashMap. Add import java.util.HashMap; to resolve compilation error.

coderabbitai · 2025-12-02T22:19:26Z

Important

Review skipped

More than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review.

44 files out of 176 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feature/ai-agent-tooling

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

Copilot reviewed 151 out of 179 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T22:23:55Z

src/main/java/ai/labs/eddi/modules/langchain/impl/LangchainTask.java

+
+        if (!isNullOrEmpty(processedParams.get(KEY_CONVERT_TO_OBJECT)) &&
+                Boolean.parseBoolean(processedParams.get(KEY_CONVERT_TO_OBJECT))) {
+            var contentAsObject = jsonSerialization.deserialize(responseContent, Map.class);


The parameter name changed from KEY_CONVERT_TO_OBJECT to responseContent, but the logic should deserialize responseContent, not read from processedParams. This appears correct, but the comment on line 287 checking KEY_CONVERT_TO_OBJECT suggests the intent is to convert based on a parameter, not always convert. The original code was jsonSerialization.deserialize(processedParams.get(KEY_CONVERT_TO_OBJECT), Map.class) which is incorrect. The fix should verify the boolean flag before attempting deserialization.

…o object

Copilot

Pull request overview

Copilot reviewed 151 out of 179 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

pom.xml:1

The Docker base image version 1.23 may not exist yet. The current stable version is 1.22 (as shown in the diff). Verify this version exists before merging.

<?xml version="1.0"?>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T22:34:35Z

src/main/java/ai/labs/eddi/modules/langchain/rest/RestToolHistory.java

+            return Response.ok(trace).build();
+
+        } catch (ai.labs.eddi.datastore.IResourceStore.ResourceNotFoundException e) {
+             return Response.status(Response.Status.NOT_FOUND)


Inconsistent spacing in Response.status() calls. Line 80 has correct spacing while line 276 has inconsistent alignment. Consider standardizing the formatting for all Response.status() calls throughout the file.

Suggested change

return Response.status(Response.Status.NOT_FOUND)

return Response.status(Response.Status.NOT_FOUND)

Copilot · 2025-12-02T22:34:35Z

src/main/java/ai/labs/eddi/modules/langchain/tools/EddiToolBridge.java

+    @Tool("Executes a pre-configured EDDI httpcall. Use only httpcalls that have been explicitly provided to you.")
+    public String executeHttpCall(String conversationId, String httpCallUri, Map<String, Object> arguments) {
+        try {
+            Method method = this.getClass().getMethod("internalExecuteHttpCall", String.class, String.class, Map.class);


The reflection-based method lookup happens on every tool execution. Consider caching the Method object as a static final field to avoid repeated reflection overhead.

Copilot · 2025-12-02T22:34:36Z

src/main/java/ai/labs/eddi/modules/httpcalls/impl/HttpCallsTask.java

-            BatchRequestBuildingInstruction batchRequest = preRequest.getBatchRequests();
-            if (batchRequest.getExecuteCallsSequentially() == null) {
-                batchRequest.setExecuteCallsSequentially(false);
+                httpCallExecutor.execute(call, memory, templateDataObjects, httpCallsConfig.getTargetServerUrl());


Return value from httpCallExecutor.execute() is ignored. This breaks the execution flow for HTTP calls that need to process responses, store results, or trigger post-response actions. The original code properly handled responses through the full execution loop.

Suggested change

httpCallExecutor.execute(call, memory, templateDataObjects, httpCallsConfig.getTargetServerUrl());

var httpCallResult = httpCallExecutor.execute(call, memory, templateDataObjects, httpCallsConfig.getTargetServerUrl());

currentStep.storeData(new IData<>("httpCallResult", httpCallResult));

Copilot · 2025-12-02T22:34:36Z

src/main/java/ai/labs/eddi/modules/langchain/impl/AgentExecutionHelper.java

+     * Determines if an error is retryable
+     */
+    private static boolean isRetryableError(Exception e) {
+        // Check for specific exception types first
+        if (e instanceof java.net.SocketTimeoutException || 
+            e instanceof java.util.concurrent.TimeoutException) {
+            return true;
+        }
+
+        String message = e.getMessage() != null ? e.getMessage().toLowerCase() : "";
+
+        // Retry on common transient errors
+        // Note: String matching is fragile and locale-dependent, but necessary as a fallback
+        // for exceptions that don't have specific types or wrap underlying errors.
+        return message.contains("timeout") ||
+               message.contains("rate limit") ||
+               message.contains("too many requests") ||
+               message.contains("503") ||
+               message.contains("502") ||
+               message.contains("504") ||
+               message.contains("connection") ||
+               message.contains("temporary");


String matching for error detection is fragile and locale-dependent. The comment on line 180-181 acknowledges this but doesn't address it. Consider defining an enum of retryable error types or using exception cause chains to determine retryability more reliably.

Suggested change

* Determines if an error is retryable

*/

private static boolean isRetryableError(Exception e) {

// Check for specific exception types first

if (e instanceof java.net.SocketTimeoutException ||

e instanceof java.util.concurrent.TimeoutException) {

return true;

}

String message = e.getMessage() != null ? e.getMessage().toLowerCase() : "";

// Retry on common transient errors

// Note: String matching is fragile and locale-dependent, but necessary as a fallback

// for exceptions that don't have specific types or wrap underlying errors.

return message.contains("timeout") ||

message.contains("rate limit") ||

message.contains("too many requests") ||

message.contains("503") ||

message.contains("502") ||

message.contains("504") ||

message.contains("connection") ||

message.contains("temporary");

* Enum of known retryable error types.

*/

private enum RetryableErrorType {

SOCKET_TIMEOUT(java.net.SocketTimeoutException.class),

TIMEOUT(java.util.concurrent.TimeoutException.class),

// Add more known retryable exception types here as needed

;

private final Class<? extends Throwable> exceptionClass;

RetryableErrorType(Class<? extends Throwable> exceptionClass) {

this.exceptionClass = exceptionClass;

}

public boolean matches(Throwable t) {

return exceptionClass.isInstance(t);

}

}

/**

* Determines if an error is retryable by checking known types and traversing the cause chain.

*/

private static boolean isRetryableError(Exception e) {

Throwable current = e;

while (current != null) {

// Check for known retryable exception types

for (RetryableErrorType type : RetryableErrorType.values()) {

if (type.matches(current)) {

return true;

}

}

// Fallback: string matching on message

String message = current.getMessage() != null ? current.getMessage().toLowerCase() : "";

if (message.contains("timeout") ||

message.contains("rate limit") ||

message.contains("too many requests") ||

message.contains("503") ||

message.contains("502") ||

message.contains("504") ||

message.contains("connection") ||

message.contains("temporary")) {

return true;

}

current = current.getCause();

}

return false;

Copilot · 2025-12-02T22:34:36Z

docs/semantic-parser.md

-| Regular     | `eddi://ai.labs.parser.dictionaries.regular`       | URI to a regular dictionary resource: `eddi://ai.labs.regulardictionary/regulardictionarystore/regulardictionaries/<`**UNIQUE\_ID**`>version <`**VERSION**`>`                                                                                                                                                                                 |
+**Problem**: Corrections too aggressive (wrong routing)  
+**Solution**: Reduce Levenshtein distance or disable specific corrections



The troubleshooting section references 'Levenshtein distance' but doesn't explain how to configure it. Add a reference to the configuration section or provide an example configuration snippet.

Suggested change

> **Example:** To reduce the Levenshtein distance threshold for fuzzy matching, set the value in your pattern matcher configuration (e.g., in `pattern-matcher.yaml`):

>

> ```yaml

> fuzzy_matching:

> levenshtein_distance: 1

> ```

Copilot · 2025-12-02T22:34:37Z

docs/langchain.md


-Sure, here is the extended section for the configuration options:
+**Note**: Additional agent mode features like `maxBudgetPerConversation`, `enableToolCaching`, `enableRateLimiting`, and custom HTTP call tools are defined in the configuration model but not yet fully implemented in the code execution path.


This note indicates that advertised features are not implemented. Either remove these unimplemented features from the configuration documentation or add clear 'Coming Soon' labels throughout to prevent user confusion.

Suggested change

Sure, here is the extended section for the configuration options:

**Note**: Additional agent mode features like `maxBudgetPerConversation`, `enableToolCaching`, `enableRateLimiting`, and custom HTTP call tools are defined in the configuration model but not yet fully implemented in the code execution path.

| **(Coming Soon)** ||||

| `maxBudgetPerConversation` | number | **Coming Soon:** Limit total tool/LLM usage per conversation | N/A |

| `enableToolCaching` | boolean | **Coming Soon:** Cache tool results to reduce API calls | N/A |

| `enableRateLimiting` | boolean | **Coming Soon:** Limit tool/LLM usage rate | N/A |

| `customHttpCallTools` | object[] | **Coming Soon:** Define custom HTTP call tools | N/A |

**Note**: The above features marked "Coming Soon" are defined in the configuration model but not yet fully implemented in the code execution path.

…gchain and HTTP calls

Copilot

Pull request overview

Copilot reviewed 151 out of 179 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T22:51:03Z

src/main/java/ai/labs/eddi/modules/langchain/tools/EddiToolBridge.java

+            INTERNAL_EXECUTE_METHOD = EddiToolBridge.class.getMethod("internalExecuteHttpCall", String.class, String.class, Map.class);
+        } catch (NoSuchMethodException e) {
+            throw new ExceptionInInitializerError("Failed to cache internalExecuteHttpCall method: " + e.getMessage());


[nitpick] The method name 'internalExecuteHttpCall' is inconsistent with the exposed method 'executeHttpCall'. Consider renaming to 'doExecuteHttpCall' or 'executeHttpCallInternal' to follow common Java naming conventions for internal helper methods.

Suggested change

INTERNAL_EXECUTE_METHOD = EddiToolBridge.class.getMethod("internalExecuteHttpCall", String.class, String.class, Map.class);

} catch (NoSuchMethodException e) {

throw new ExceptionInInitializerError("Failed to cache internalExecuteHttpCall method: " + e.getMessage());

INTERNAL_EXECUTE_METHOD = EddiToolBridge.class.getMethod("executeHttpCallInternal", String.class, String.class, Map.class);

} catch (NoSuchMethodException e) {

throw new ExceptionInInitializerError("Failed to cache executeHttpCallInternal method: " + e.getMessage());

Copilot · 2025-12-02T22:51:03Z

src/main/java/ai/labs/eddi/modules/langchain/rest/RestToolHistory.java

+                        if (data.getKey() != null && data.getKey().startsWith("langchain:trace:")) {
+                            Object result = data.getResult();
+                            if (result instanceof List) {
+                                List<Map<String, Object>> stepTrace = (List<Map<String, Object>>) result;


Unchecked cast warning. Consider adding @SuppressWarnings("unchecked") at the method level or adding type checking before casting to improve type safety.

Suggested change

List<Map<String, Object>> stepTrace = (List<Map<String, Object>>) result;

List<?> rawList = (List<?>) result;

List<Map<String, Object>> stepTrace = new ArrayList<>();

for (Object item : rawList) {

if (item instanceof Map) {

@SuppressWarnings("unchecked")

Map<String, Object> mapItem = (Map<String, Object>) item;

stepTrace.add(mapItem);

} else {

LOGGER.warn("Unexpected item type in tool trace list: " + item.getClass());

}

}

Copilot · 2025-12-02T22:51:03Z

src/main/java/ai/labs/eddi/modules/langchain/model/LangChainConfiguration.java

+         * Enable built-in tools (calculator, web search, datetime, etc.)
+         * Default: false (opt-in for security)
+         */
+        private Boolean enableBuiltInTools = false;


[nitpick] Using Boolean wrapper type for a field with a default value. Consider using primitive 'boolean' type instead, as the field already has a default value and null handling doesn't appear necessary here.

Suggested change

private Boolean enableBuiltInTools = false;

private boolean enableBuiltInTools = false;

Copilot · 2025-12-02T22:51:04Z

src/main/java/ai/labs/eddi/modules/langchain/impl/LangchainTask.java

+        // Actually, chatMessages includes the last user message.
+        // So we want to capture everything *after* the messages we seeded.


These implementation comments should be removed or clarified before production. They appear to be developer notes that don't add value for future maintainers and may indicate uncertainty in the implementation logic.

Suggested change

// Actually, chatMessages includes the last user message.

// So we want to capture everything *after* the messages we seeded.

// chatMessages includes the last user message.

// Capture all messages added after the seeded history (i.e., after the last user message).

Copilot · 2025-12-02T22:51:04Z

src/main/java/ai/labs/eddi/modules/httpcalls/impl/HttpCallsTask.java

+                var httpCallResult = httpCallExecutor.execute(call, memory, templateDataObjects, httpCallsConfig.getTargetServerUrl());
+                // Result is already stored in memory by HttpCallExecutor via prePostUtils
+                // The returned map can be used for additional processing if needed
+                if (httpCallResult != null && !httpCallResult.isEmpty()) {
+                    templateDataObjects.putAll(httpCallResult);


The comment states 'Result is already stored in memory' but the code still adds it to templateDataObjects. This creates confusion about whether the result is being double-stored or if the comment is misleading. Clarify the purpose of this additional storage.

…ty in Langchain and HttpCallsTask

Copilot

Pull request overview

Copilot reviewed 151 out of 179 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T23:06:16Z

src/main/java/ai/labs/eddi/modules/langchain/tools/EddiToolBridge.java

+            // Load the conversation memory snapshot
+            ConversationMemorySnapshot snapshot = conversationMemoryStore.loadConversationMemorySnapshot(conversationId);
+
+            // Create a temporary memory instance to satisfy HttpCallExecutor contract
+            ConversationMemory memory = new ConversationMemory(conversationId, snapshot.getBotId(), snapshot.getBotVersion(), snapshot.getUserId());
+


Creating a temporary ConversationMemory instance from a snapshot may lead to incomplete memory state. Consider passing the actual IConversationMemory instance through the execution context or retrieving it from the conversationMemoryStore instead of reconstructing it from the snapshot.

Suggested change

// Load the conversation memory snapshot

ConversationMemorySnapshot snapshot = conversationMemoryStore.loadConversationMemorySnapshot(conversationId);

// Create a temporary memory instance to satisfy HttpCallExecutor contract

ConversationMemory memory = new ConversationMemory(conversationId, snapshot.getBotId(), snapshot.getBotVersion(), snapshot.getUserId());

// Load the actual conversation memory instance

ConversationMemory memory = conversationMemoryStore.loadConversationMemory(conversationId);

Copilot · 2025-12-02T23:06:16Z

src/main/java/ai/labs/eddi/modules/langchain/rest/RestToolHistory.java

+                toolCalls.add(currentCall);
+            } else if ("tool_result".equals(type) && currentCall != null) {
+                // Match with last call - simplistic but works for sequential execution
+                if (currentCall.getToolName().equals(event.get("tool"))) {


Potential NullPointerException if currentCall.getToolName() or event.get("tool") returns null. Add null checks before the equals comparison.

Copilot · 2025-12-02T23:06:17Z

src/main/java/ai/labs/eddi/modules/langchain/model/ToolExecutionTrace.java

+        sb.append("Cache Hit Rate: ").append(
+            toolCalls.size() > 0 ? String.format("%.1f%%", (double) cacheHits / toolCalls.size() * 100) : "0%"
+        ).append("\n");


The cache hit rate calculation uses toolCalls.size() which includes all calls, but cacheHits is only incremented for successful calls. Consider using a denominator that only counts cacheable calls (successful + cached) for a more accurate hit rate.

Copilot · 2025-12-02T23:06:17Z

src/main/java/ai/labs/eddi/modules/langchain/impl/AgentExecutionHelper.java

+            // Fallback: string matching on message
+            String message = current.getMessage() != null ? current.getMessage().toLowerCase() : "";
+
+            if (message.contains("timeout") ||
+                message.contains("rate limit") ||
+                message.contains("too many requests") ||
+                message.contains("503") ||
+                message.contains("502") ||
+                message.contains("504") ||
+                message.contains("connection") ||
+                message.contains("temporary")) {
+                return true;
+            }
+


String-based error detection using contains() is fragile and may lead to false positives (e.g., 'The user mentioned 503 Service Unavailable in their message'). Consider defining specific exception types or error codes instead of relying on substring matching.

Suggested change

// Fallback: string matching on message

String message = current.getMessage() != null ? current.getMessage().toLowerCase() : "";

if (message.contains("timeout") ||

message.contains("rate limit") ||

message.contains("too many requests") ||

message.contains("503") ||

message.contains("502") ||

message.contains("504") ||

message.contains("connection") ||

message.contains("temporary")) {

return true;

}

// Removed fallback string matching on message for retryable error detection.

Copilot · 2025-12-02T23:06:17Z

src/main/java/ai/labs/eddi/modules/httpcalls/impl/HttpCallsTask.java

+                var httpCallResult = httpCallExecutor.execute(call, memory, templateDataObjects, httpCallsConfig.getTargetServerUrl());
+                // HttpCallExecutor stores response in conversation memory via prePostUtils.
+                // We also merge into templateDataObjects so subsequent calls in this loop can reference previous results.
+                if (httpCallResult != null && !httpCallResult.isEmpty()) {
+                    templateDataObjects.putAll(httpCallResult);
                }


Merging httpCallResult into templateDataObjects without key prefixing may cause key collisions if multiple HTTP calls return the same keys. Consider prefixing keys with the call name (e.g., 'call1.body', 'call2.body') to avoid overwriting data.

Copilot · 2025-12-02T23:06:18Z

src/main/java/ai/labs/eddi/modules/httpcalls/impl/HttpCallExecutor.java

+                                        String targetServerUrl) throws LifecycleException {
+        if (memory == null) {
+            throw new IllegalArgumentException("memory cannot be null");
+        }


The null check for memory is good, but other parameters (call, templateDataObjects, targetServerUrl) are not validated. Consider adding validation for all required parameters to fail fast and provide clear error messages.

Suggested change

}

}

if (call == null) {

throw new IllegalArgumentException("call cannot be null");

}

if (templateDataObjects == null) {

throw new IllegalArgumentException("templateDataObjects cannot be null");

}

if (targetServerUrl == null || targetServerUrl.trim().isEmpty()) {

throw new IllegalArgumentException("targetServerUrl cannot be null or empty");

}

…lean for nullable support

…nd improve error handling in RestToolHistory

ginccc requested a review from Copilot December 2, 2025 18:04

labsai deleted a comment from coderabbitai bot Dec 2, 2025

Copilot AI reviewed Dec 2, 2025

View reviewed changes

ginccc added 4 commits December 2, 2025 19:16

feat(docs): Enhance tool configuration guidance for Web Search and We…

b79e929

…ather tools

feat(langchain): Update tool history processing to use PackageRunSnap…

eecd1b6

…shot and ResultSnapshot

feat(tests): Update EddiToolBridgeTest to allow any parameter for htt…

34c7872

…pCallExecutor execution

labsai deleted a comment from coderabbitai bot Dec 2, 2025

ginccc requested a review from Copilot December 2, 2025 20:32

Copilot AI reviewed Dec 2, 2025

View reviewed changes

feat(tools): Update EddiToolBridge to use ConcurrentHashMap for httpC…

6e7d647

…all configuration caching

labsai deleted a comment from coderabbitai bot Dec 2, 2025

ginccc requested a review from Copilot December 2, 2025 21:44

Copilot AI reviewed Dec 2, 2025

View reviewed changes

feat(tools): Added HashMap import to EddiToolBridge

8bf25ec

ginccc requested a review from Copilot December 2, 2025 22:23

Copilot AI reviewed Dec 2, 2025

View reviewed changes

feat(langchain): Simplify condition for converting response content t…

f748e4a

…o object

ginccc requested a review from Copilot December 2, 2025 22:33

Copilot AI reviewed Dec 2, 2025

View reviewed changes

feat(langchain): Enhance tool configuration and error handling in Lan…

a75eb8d

…gchain and HTTP calls

ginccc requested a review from Copilot December 2, 2025 22:49

Copilot AI reviewed Dec 2, 2025

View reviewed changes

feat(langchain): Refactor method names and improve comments for clari…

0fe7478

…ty in Langchain and HttpCallsTask

ginccc requested a review from Copilot December 2, 2025 23:04

Copilot AI reviewed Dec 2, 2025

View reviewed changes

ginccc added 2 commits December 3, 2025 00:10

feat(langchain): Reverted enableBuiltInTools type from boolean to Boo…

cb5c49c

…lean for nullable support

feat(httpcalls): Add null checks for parameters in HttpCallExecutor a…

b304ca1

…nd improve error handling in RestToolHistory

		private static boolean isRetryableError(Exception e) {
		String message = e.getMessage() != null ? e.getMessage().toLowerCase() : "";

	return Response.status(Response.Status.NOT_FOUND)
	return Response.status(Response.Status.NOT_FOUND)

	httpCallExecutor.execute(call, memory, templateDataObjects, httpCallsConfig.getTargetServerUrl());
	var httpCallResult = httpCallExecutor.execute(call, memory, templateDataObjects, httpCallsConfig.getTargetServerUrl());
	currentStep.storeData(new IData<>("httpCallResult", httpCallResult));

+> **Example:** To reduce the Levenshtein distance threshold for fuzzy matching, set the value in your pattern matcher configuration (e.g., in `pattern-matcher.yaml`):
+>
+> ```yaml
+> fuzzy_matching:
+>   levenshtein_distance: 1
+> ```


		Sure, here is the extended section for the configuration options:
		Note: Additional agent mode features like `maxBudgetPerConversation`, `enableToolCaching`, `enableRateLimiting`, and custom HTTP call tools are defined in the configuration model but not yet fully implemented in the code execution path.

-Sure, here is the extended section for the configuration options:
-**Note**: Additional agent mode features like `maxBudgetPerConversation`, `enableToolCaching`, `enableRateLimiting`, and custom HTTP call tools are defined in the configuration model but not yet fully implemented in the code execution path.
+| **(Coming Soon)** ||||
+| `maxBudgetPerConversation` | number | **Coming Soon:** Limit total tool/LLM usage per conversation | N/A |
+| `enableToolCaching` | boolean | **Coming Soon:** Cache tool results to reduce API calls | N/A |
+| `enableRateLimiting` | boolean | **Coming Soon:** Limit tool/LLM usage rate | N/A |
+| `customHttpCallTools` | object[] | **Coming Soon:** Define custom HTTP call tools | N/A |
+**Note**: The above features marked "Coming Soon" are defined in the configuration model but not yet fully implemented in the code execution path.

-                                List<Map<String, Object>> stepTrace = (List<Map<String, Object>>) result;
+                                List<?> rawList = (List<?>) result;
+                                List<Map<String, Object>> stepTrace = new ArrayList<>();
+                                for (Object item : rawList) {
+                                    if (item instanceof Map) {
+                                        @SuppressWarnings("unchecked")
+                                        Map<String, Object> mapItem = (Map<String, Object>) item;
+                                        stepTrace.add(mapItem);
+                                    } else {
+                                        LOGGER.warn("Unexpected item type in tool trace list: " + item.getClass());
+                                    }
+                                }

	private Boolean enableBuiltInTools = false;
	private boolean enableBuiltInTools = false;

		// Actually, chatMessages includes the last user message.
		// So we want to capture everything after the messages we seeded.

-        }
+        }
+        if (call == null) {
+            throw new IllegalArgumentException("call cannot be null");
+        }
+        if (templateDataObjects == null) {
+            throw new IllegalArgumentException("templateDataObjects cannot be null");
+        }
+        if (targetServerUrl == null || targetServerUrl.trim().isEmpty()) {
+            throw new IllegalArgumentException("targetServerUrl cannot be null or empty");
+        }

feat(langchain): Add comprehensive AI Agent and Tooling support #389

Are you sure you want to change the base?

feat(langchain): Add comprehensive AI Agent and Tooling support #389

Uh oh!

Conversation

ginccc commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Dec 2, 2025

Review skipped

Other AI code review bot(s) detected

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

ginccc commented Dec 2, 2025 •

edited

Loading