alnzng commented on code in PR #1114:
URL: https://github.com/apache/flink-agents/pull/1114#discussion_r4022357973
##########
python/flink_agents/api/subagent.py:
##########
@@ -15,18 +15,68 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#################################################################################
+import json
import logging
from abc import ABC, abstractmethod
from dataclasses import dataclass
from typing import TYPE_CHECKING, Any
+from pydantic import BaseModel, field_validator, model_validator
+
from flink_agents.api.resource import ResourceType, SerializableResource
if TYPE_CHECKING:
from flink_agents.api.runner_context import RunnerContext
_LOG = logging.getLogger(__name__)
+# Prefix of the callable name a sub-agent is exposed to a chat model under.
+# Tools are forbidden to register under this prefix, so a prefixed callable
+# name unambiguously addresses a sub-agent and the executing side routes it to
+# the AGENT namespace.
+CALLABLE_NAME_PREFIX = "subagent_"
Review Comment:
if this is intended to be internal only, I would prefer using "_subagent_"
as prefix to keep consistent with existing patterns, e.g. `_TOOL_CALL_CONTEXT`
in chat_model_action.py
##########
plan/src/main/java/org/apache/flink/agents/plan/actions/ToolCallAction.java:
##########
@@ -197,21 +218,32 @@ private static void executeParallel(
Map<String, Boolean> success,
Map<String, String> error,
Map<String, ToolResponse> responses) {
- List<DurableCallable<ToolResponse>> callables = new
ArrayList<>(executions.size());
+ // Sub-agent calls already run through durable execution inside the
setup, so they stay
+ // synchronous here and only the tool calls enter the durable batch.
+ List<ToolCallExecution> toolExecutions = new ArrayList<>();
for (ToolCallExecution execution : executions) {
+ if (execution.agent != null) {
+ dispatchAgentExecution(execution, ctx, success, error,
responses);
Review Comment:
It looks like all the sub-agent tool calls not run parallel, this is
different with the regular tool call execution, right? any reason behind this?
##########
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:
I wonder why we decided to drop sub-agent silently, not fail the job
directly.
--
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]
