gnodet-bot commented on code in PR #28354:
URL: https://github.com/apache/camel/pull/28354#discussion_r4368418810
##########
components/camel-ai/camel-langchain4j-agent-api/src/main/java/org/apache/camel/component/langchain4j/agent/api/AbstractAgent.java:
##########
@@ -55,6 +60,60 @@ public abstract class AbstractAgent<S> implements Agent {
protected AbstractAgent(AgentConfiguration configuration) {
this.configuration = configuration;
+ validateToolSearchCompensation(configuration);
+ }
+
+ /**
+ * LangChain4j's tools(List<AiServiceTool>) overload does not register
+ *
+ * @CompensateFor methods. Reject this unsupported combination before the
agent starts handling requests.
Review Comment:
🔵 **Malformed Javadoc tag — `@CompensateFor` is parsed as a block tag, not
literal text.**
The Javadoc processor treats `@CompensateFor` at the start of a line as a
custom Javadoc block tag (like `@param` or `@return`), splitting the sentence
across the main description and an unknown-tag section. Use `{@code
@CompensateFor}` for literal annotation names:
```suggestion
/**
* LangChain4j's {@code tools(List<AiServiceTool>)} overload does not
register
* {@code @CompensateFor} methods. Reject this unsupported combination
before the agent starts handling requests.
*/
```
<!-- generated by an AI agent, Hermès -->
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -355,6 +421,13 @@ private ToolProvider createComposedToolProvider(String
tags, Exchange exchange,
}
}
+ private static void rejectReservedToolName(ToolSpecification
specification) {
+ if (AiToolSearch.TOOL_NAME.equals(specification.name())) {
+ throw new IllegalArgumentException(
+ "Tool name '" + AiToolSearch.TOOL_NAME + "' is reserved
while toolSearch=true");
+ }
+ }
+
Review Comment:
🔵 **Dead code — `rejectReservedToolName(ToolSpecification)` is never
called.**
The instance method `rejectReservedToolName(LangChain4jAgentConfiguration)`
at line 324 is called from `doInit()`. This static overload
`rejectReservedToolName(ToolSpecification)` has no callers — the MCP-side check
now uses the `AtomicBoolean` filter pattern instead. Remove it.
```suggestion
```
<!-- generated by an AI agent, Hermès -->
##########
components/camel-ai/camel-ai-tool/src/main/java/org/apache/camel/component/ai/tool/AiToolSearchSessions.java:
##########
@@ -0,0 +1,234 @@
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.ai.tool;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import java.util.function.LongSupplier;
+
+/**
+ * Conversation-scoped {@link AiToolSearchSession}s.
+ * <p>
+ * Sessions are retained only after a tool has been discovered. Retained
sessions expire after a period of inactivity
+ * and are subject to a maximum size. Access and mutation are synchronized so
expiration and capacity eviction cannot
+ * corrupt the session catalogue.
+ * <p>
+ * Expired entries are removed during subsequent operations; no background
thread is required. Callers can explicitly
+ * remove a conversation or clear all retained sessions when their
conversation or chat-memory lifecycle ends.
+ *
+ * @since 4.24
+ */
+public final class AiToolSearchSessions {
+
+ static final int DEFAULT_MAX_SESSIONS = 10_000;
+ static final long DEFAULT_EXPIRATION_MILLIS =
TimeUnit.MINUTES.toMillis(30);
+
+ private final int maxSessions;
+ private final long expirationNanos;
+ private final LongSupplier nanoClock;
+
+ /*
+ * Access-order LinkedHashMap provides least-recently-used eviction.
+ * All access is guarded by this instance's monitor.
+ */
+ private final LinkedHashMap<String, Entry> sessions = new
LinkedHashMap<>(16, 0.75f, true);
+
+ public AiToolSearchSessions() {
+ this(DEFAULT_MAX_SESSIONS, DEFAULT_EXPIRATION_MILLIS,
System::nanoTime);
+ }
+
+ AiToolSearchSessions(int maxSessions, long expirationMillis) {
+ this(maxSessions, expirationMillis, System::nanoTime);
+ }
+
+ AiToolSearchSessions(int maxSessions, long expirationMillis, LongSupplier
nanoClock) {
+ if (maxSessions < 1) {
+ throw new IllegalArgumentException("maxSessions must be greater
than zero");
+ }
+ if (expirationMillis < 1) {
+ throw new IllegalArgumentException("expirationMillis must be
greater than zero");
+ }
+ this.maxSessions = maxSessions;
+ this.expirationNanos = TimeUnit.MILLISECONDS.toNanos(expirationMillis);
+ this.nanoClock = Objects.requireNonNull(nanoClock, "nanoClock");
+ }
+
+ /**
+ * Returns the session for a conversation id.
+ * <p>
+ * An unregistered session is returned without retaining it. It is
registered only when a tool is remembered.
+ *
+ * @param conversationId stable id for one conversation; must not be blank
+ * @return the session for that id
+ */
+ public synchronized AiToolSearchSession session(String conversationId) {
+ validateConversationId(conversationId);
+
+ long now = nanoClock.getAsLong();
+ purgeExpired(now);
+
+ Entry entry = sessions.get(conversationId);
+ if (entry != null) {
+ entry.lastAccessNanos = now;
+ return entry.session;
+ }
+
+ return new AiToolSearchSession(this, conversationId);
+ }
+
+ /**
+ * Removes the retained session for a conversation.
+ *
+ * @param conversationId conversation id
+ * @return true if a retained session was removed
+ */
+ /**
+ * Finds an existing retained session without creating a new one.
+ */
+ public synchronized AiToolSearchSession find(String conversationId) {
Review Comment:
🔵 **Dangling `remove()` Javadoc stranded above `find()`.**
Lines 101-106 contain the Javadoc for `remove()` (`@param conversationId`,
`@return true if a retained session was removed`), but `find()` was inserted
between it and `remove()`. As-is:
- `find()` inherits the wrong Javadoc
- `remove()` at line 126 has no Javadoc at all
Move the `remove()` Javadoc block to immediately precede `remove()`, and
give `find()` a proper doc:
```suggestion
/**
* Finds an existing retained session without creating a new one.
*
* @param conversationId conversation id; returns {@code null} when
blank or unknown
* @return the retained session, or {@code null} if
absent or expired
*/
public synchronized AiToolSearchSession find(String conversationId) {
```
<!-- generated by an AI agent, Hermès -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
