yunfengzhou-hub commented on code in PR #1114:
URL: https://github.com/apache/flink-agents/pull/1114#discussion_r4026739337


##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelSetup.java:
##########
@@ -104,11 +116,54 @@ public void open() throws Exception {
             }
             this.toolNames = mutable;
         }
+        // Rebuilt from scratch: open() may run again on the same instance, 
and the callables must
+        // not accumulate.
+        this.tools.clear();
+        Set<String> callableNames = new LinkedHashSet<>();
         if (this.toolNames != null) {
             for (String name : this.toolNames) {
+                Preconditions.checkState(
+                        callableNames.add(name), "Duplicate callable name: 
%s", name);
                 this.tools.add((Tool) this.resourceContext.getResource(name, 
ResourceType.TOOL));
             }
         }
+        for (String name : this.subagentNames) {
+            // Tools are forbidden to carry the reserved prefix at 
registration, so a prefixed
+            // callable name can only come from this loop and a clash with a 
tool is impossible.
+            // Checked before the schema below, because a name declared twice 
is a mistake in the
+            // declaration whether or not it ends up registered.
+            Preconditions.checkState(
+                    callableNames.add(SubagentSetup.CALLABLE_NAME_PREFIX + 
name),
+                    "Duplicate callable name: %s",
+                    SubagentSetup.CALLABLE_NAME_PREFIX + name);
+            Resource resource = this.resourceContext.getResource(name, 
ResourceType.AGENT);
+            // A sub-agent owned by the other language resolves to a bridge 
handle here, which
+            // carries no schema to declare, so it is rejected instead of 
silently dropped.
+            Preconditions.checkState(
+                    resource instanceof SubagentSetup,
+                    "Sub-agent %s must resolve to a SubagentSetup, but was %s",
+                    name,
+                    resource.getClass().getName());
+            SubagentSetup setup = (SubagentSetup) resource;
+            String inputSchema = setup.getInputSchema();
+            if (inputSchema == null) {
+                // Unlike a bridge handle this is a sub-agent the caller could 
have described, so
+                // it is dropped with a warning rather than failing the job: 
the rest of the

Review Comment:
   It was supposed that we may have some built-in sub-agents that may not be 
exposed to LLM, which means they do not need schemas. But this assumption is 
not that solid so I agree to reject this corner case for now.



-- 
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]

Reply via email to