Repository navigation
[api][runtime][python] Support running a ReActAgent as a general-purpose sub-agent - #1206
Conversation
7bdfdb9 to
c2b87d2
Compare
22e27c0 to
7af619d
Compare
| prompt: Prompt | None = None, | ||
| output_schema: type[BaseModel] | RowTypeInfo | None = None, | ||
| subagent_description: str | None = None, | ||
| subagent_input_schema: str | None = None, |
There was a problem hiding this comment.
Should-fix: overriding subagent_description (or supplying a prompt) without
also overriding subagent_input_schema can silently produce a wrong prompt.
The default schema declares a single input string field, so the parent model
always calls the child with {"input": ...}. If the child is constructed with a
prompt whose placeholders are not input (e.g. {diff}), start_action takes the
prompt branch and calls prompt.format_messages(input=...). Because SafeFormatter
returns the literal {diff} for a missing key instead of raising, the model receives
a prompt with an unreplaced {diff} — no error, just a broken delegation.
Could we fail fast (or warn) at construction time when a prompt is provided whose
placeholders don't match the declared/default input schema? Or derive the default
schema from the prompt's placeholders. At minimum, the subagent_input_schema
docstring should call out this coupling explicitly.
There was a problem hiding this comment.
Thanks for catching this. I think it's better to document the prompt/schema contract rather than add construction-time validation: schema field names must match the child prompt placeholders, while prompt-free model-driven delegation requires an explicit single input field. Reliable validation is not generally possible because prompts and JSON Schemas can express shapes that cannot be matched statically.
| gen.writeStringField("description", provider.getDescription()); | ||
| } | ||
| if (provider.getInputSchema() != null) { | ||
| gen.writeStringField("inputSchema", provider.getInputSchema()); |
There was a problem hiding this comment.
Should-fix (cross-language, relevant to #1141): the metadata field name and the
Java→Python conversion path are asymmetric.
-
Field name: Java serializes
inputSchema(camelCase, here), while the Python
compiler writesinput_schema(snake_case,agent_plan.py:532). A plan JSON
produced by one side is not read back with the same key by the other. -
The Python
__custom_deserializebranch that converts a Java-emitted
InternalSubagentProviderinto aPythonSerializableResourceProvider
(agent_plan.py:152-168) only carrieschild_planandscope— it drops
description/inputSchemaentirely. So a Java-compiled sub-agent that reaches
Python loses the metadata this PR adds.
Neither bites a single-language job, but both will break model-driven delegation for
a cross-language sub-agent. Worth aligning the key name and carrying the two fields
through the conversion before #1141 lands on top of this.
There was a problem hiding this comment.
Fixed. The cross-language key is now consistently input_schema, and both description and input_schema are preserved when converting an internal sub-agent provider in either direction. I also added Java/Python round-trip and cross-language regression coverage.
|
|
||
| {{< /tabs >}} | ||
|
|
||
| The awaited value is a `SubagentResult`. On success, its `result` field (`getResult()` in Java) is the list of outputs the child emitted — a ReActAgent child emits a single final answer, so the first element is what you usually want. On failure, `success` is `False` / `isSuccess()` is `false`, and `error_message` / `getErrorMessage()` carries the reason. |
There was a problem hiding this comment.
Question: for model-driven delegation the child's answer reaches the parent model
as a list-wrapped observation (e.g. ["reviewed -> HELLO WORLD!"]), because
SubagentResult.result is the list of outputs the child emitted and
to_chat_message_content JSON-encodes it as-is.
The docs here correctly tell a caller-driven user to take result.result[0], but in
the model-driven path the model sees the raw JSON list. For a ReActAgent child that
always emits exactly one final answer, is the list wrapper intentional in the tool
observation, or should the single-output case be flattened so the model observes the
plain string? A one-line note either way would help readers.
There was a problem hiding this comment.
The list wrapper is intentional. SubagentResult.result consistently represents all outputs emitted by the child, and flattening only the single-output case would make the observation shape depend on output cardinality. I added a note clarifying that model-driven delegation JSON-encodes the same result list as the tool observation, even for a ReActAgent that emits one final answer.
There was a problem hiding this comment.
Update: this note now lives on the sub-agents page, under "Delegate from a chat model".
| action reads its own config instead of failing on the root plan, which | ||
| does not carry that action. | ||
| """ | ||
| plan_json = self._j_runner_context.getActiveScopePlanJson() |
There was a problem hiding this comment.
Question (perf): __active_plan() crosses the py4j boundary via
getActiveScopePlanJson() on every action_config / get_action_config_value read.
The parsed plan is cached in __scoped_resource_caches, and the Java getter is cheap
(returns a cached string, no re-serialization), but the getter call itself is not
memoized, so config-heavy actions pay a JNI round-trip per lookup.
Is that acceptable on the hot path, or is it worth caching the active plan JSON
(per scope) on the Python side the first time it's seen?
There was a problem hiding this comment.
Good point. This bridge lookup was introduced to make action-config reads scope-aware, so a child action reads its own plan rather than the root plan. Parsed scoped plans are already cached; the remaining call determines which root or child scope is currently active. Caching that result in Python without an explicit scope-change signal could return stale configuration after a scope switch. Since Java returns an already cached string and config reads are infrequent compared with model calls, I would keep the correctness-first behavior here and leave a lifecycle-aware cache as a follow-up if profiling shows it is needed. What do you think?
| } | ||
|
|
||
| @Test | ||
| public void testInternalSubagentProviderRoundTrip() throws Exception { |
There was a problem hiding this comment.
Nit (commit message): commit 3e81f73 says "Also fixes a latent serializer bug the
new round-trip tests exposed: AgentPlanJsonSerializer never closed the object it
opens." But AgentPlanJsonSerializer.java is not modified in this PR — it's
byte-identical to main, which already has the closing writeEndObject() (and the
explanatory comment) from #1138; the generator is balanced (6 start / 6 end).
The round-trip tests added here are great, but the "fixes a latent serializer bug"
paragraph describes a change that isn't in this diff. Could we drop/reword that
paragraph so the commit message matches what's actually changed?
There was a problem hiding this comment.
You're right — that serializer fix was already present on main and is not part of this PR. I removed the inaccurate paragraph from the rewritten commit message.
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for adding ReActAgent sub-agent support and the caller-driven, model-driven, and tool-loop tests! The focused Java tests passed on JDK 21. I found one remaining prompt-isolation gap in the Java parent/child composition, detailed inline and reproduced with two operator-harness tests.
| try { | ||
| fields = mapper.readValue(objectMapper.writeValueAsString(input), Map.class); | ||
| } catch (JsonProcessingException e) { | ||
| if (isDefaultSubagentInputMap(input)) { |
There was a problem hiding this comment.
Could we keep ReAct's private prompt resources local to the child agent? A child constructed with prompt=null and outputSchema=null can still pick up the parent's _default_user_prompt and _default_schema_prompt: startAction() resolves these through ctx.getResource(), and the child ResourceCache falls back to the root cache when the name is absent locally. Consequently, the prompt-free path here is bypassed whenever the parent has a prompt.
I reproduced both cases through the operator harness on this head:
- Parent prompt
PARENT ONLY: {input}, bare child, delegation arguments{"input":"review the diff"}: the child model receivesPARENT ONLY: review the diffinstead ofreview the diff. - Parent configured with a POJO output schema, bare child: the child model also receives the parent's JSON-schema instruction despite having no output schema of its own.
The cache fallback predates this PR, but the newly supported ReAct parent/child composition still needs to account for it. Could we make these internal prompt lookups scope-local while preserving ordinary shared-resource lookup, and add regression tests with a configured parent and an unconfigured child?
There was a problem hiding this comment.
Fixed. ReActAgent now records whether its own action declared the built-in user prompt and output schema, and resolves those private prompt resources only when the active scope declares them. This preserves ordinary resource fallback while preventing an unconfigured child from inheriting the parent's private prompts. I added parent-configured/child-unconfigured regression coverage.
|
Thanks for the PR. I read this PR as two parts: introducing SubagentMetadataProvider for an Agent to implement so it can provide the description and schema used for model-driven invocation, and extending ReActAgent with it. My concern is about SubagentMetadataProvider. IMO, a subagent can be a component shared among agents. Its functional description may be the same, but when to use it can differ from one agent to another — a physical-machine diagnoser may also serve several distributed systems, but each of them needs different trigger symptoms, e.g. a specific metric going abnormal. Besides, I also think we'd better support registering an Agent as a subagent without requiring the Agent to be modified, or to follow some rules, for this kind of usage. From both points of view, I think we should not introduce an interface that subagents are required to implement, but instead provide the metadata at registration. WDYT? |
7af619d to
ee839e5
Compare
@pltbkd Agreed. I replaced |
|
#1242 restructures the internal sub-agent declaration and serialization paths this PR touches. I'll rebase onto that once it lands to avoid duplicating the infrastructure work; the ReActAgent-specific changes (input handling, prompt scoping, E2E coverage) are independent and will carry over. |
ee839e5 to
755268b
Compare
|
Thanks for the update! The parent/child prompt isolation issue is fixed, and the focused Java and Python tests passed. Two small documentation follow-ups:
I found no further blocking code issues. |
|
Thanks for the update. LGTM |
The start action dispatched on ClassUtils.isPrimitiveOrWrapper, which
does not cover String, so a String input failed both with and without a
user prompt, while the Python start action and the documented input
contract (development/react_agent.md lists str under the {input}
placeholder) both accept it. A Map input only worked by falling into the
Pojo branch's JSON round-trip, and that round-trip read the JSON into
the raw Map.class, so any non-String Pojo field or Map value surfaced as
a bare ClassCastException in Prompt#format, far from the input dispatch
and with no guidance, while Python stringifies every value.
Route String through the primitive branch (the {input} placeholder) and
give Map an explicit branch that stringifies keys and values, mirroring
the Python dict branch. The Pojo branch now stringifies every field the
same way instead of assigning the raw read result. Values keep their
language-native string form (String.valueOf on Java, str() on Python),
including nested ones. The unsupported-input error now lists Map among
the accepted kinds. The map-without-prompt rejection is unchanged, as
both ends reject it today.
Added a ReActAgentTest matrix covering primitive, string, map, Row and
Pojo inputs, including a Pojo with a non-String field.
Generated-by: Qoder
A model-driven call hands the child the arguments of its input schema as a map. Without a user-supplied prompt, a ReActAgent rejected every map, so a child registered with the single "input" field schema could not be called. A map with exactly one "input" entry now becomes the user message directly, mirroring the prompt-less primitive branch; any other map without a prompt still fails fast. A child scope also falls back to the root agent's resources, so an unconfigured child could pick up its parent ReActAgent's built-in user or schema prompt under the same name. Gate both lookups on the running scope's own action config, and report a declared prompt that cannot be resolved as a broken plan. Generated-by: Qoder
The internal sub-agent e2e suites only exercised hand-written echo
agents as children. Cover the built-in ReActAgent itself so its start
action, the framework's chat model action and its stop action all run
inside the child scope, driven by scripted connections instead of a
live model.
Both delegation paths are covered on each side: a caller-driven path
where a root action submits a string prompt and awaits the future, and
a model-driven path where the parent model issues a _subagent_child
tool call whose {"input": ...} arguments flow through the prompt-free
map path; the child's reply returns as the tool observation that closes
the loop. The caller-driven Python scenario also drives a child tool
loop, and the Java harness covers the parent prompt leaks.
The new Python caller-driven test also caught a latent scope bug:
FlinkRunnerContext resolved action_config and get_action_config_value
against the root plan even inside a child scope, so the child's
ReActAgent.start_action crashed reading its own output_schema config.
Mirror the Java context's scope-aware currentPlan() by resolving both
against the active scope's plan.
Generated-by: Qoder
The sub-agents page covered declaring a child but not calling it, and the ReAct agent page had no recipe for running a ReActAgent as a child. Extend the sub-agents page with delegating from a chat model, calling from an action and the SubagentResult both paths return, and a section on using a ReActAgent as the child: registering it through InternalSubagentResourceDescriptorBuilder and matching the input schema to the child's prompt. The ReAct agent page links there. Generated-by: Qoder
755268b to
f42dda0
Compare
Linked issue: #1139
Purpose of change
A ReActAgent can now run as an internal sub-agent child. It is registered on the parent as an
AGENTresource through the descriptor-basedInternalSubagentResourceDescriptorBuilderfrom #1242, which carries the registration's description and input schema. The parent can then call it from an action (submit/awaitaSubagentResult) or let its chat model delegate to it as the_subagent_<name>tool. Java and Python are at parity.#1242 provides the registration and the cross-language metadata; this PR makes the ReActAgent itself work as the child: it accepts the inputs both paths hand it, keeps to its own prompt and output schema inside the child scope, and is covered end to end in both languages.
Runtime flow
InternalSubagentResourceDescriptorBuilder().agent(...).description(...).input_schema(...); the metadata belongs to the registration, so the same agent can be registered under different names and schemas.{input}placeholder. A model-driven call passes the tool arguments as a map: with a prompt, the keys fill its placeholders; without one, a map holding onlyinputbecomes the user message.has_user_prompt,output_schema).SubagentResult.result, or to the parent model as the tool observation.Key decisions
inputmap is accepted. It is the shape the documented input schema produces; any other map still fails fast, since there is no prompt to format it.currentPlan(), so a child action reads its own config instead of failing on the root plan.Behavioral Semantics
Interaction decisions
{input}{"input": ...}accepted; value becomes the user messageClassCastException)Behavioral contracts
SubagentResultholds the child's outputs.Failure behavior
inputmap, raises as before.SubagentResultwithsuccess = falseand an error message (existing internal sub-agent semantics).Tests
ReActAgentTest#testStringInput*,#testMapInput*,#testPojoWithNonStringFieldFillsPlaceholdersWithStringifiedValue,#testInputOfUnsupportedKindReportsAcceptedKindsinputmap without a promptReActAgentTest#testSingleInputMapWithoutPromptBecomesUserMessage,#testMultiKeyMapWithoutPromptStillReportsPromptRequirement; Pythontest_single_input_map_*,test_multi_key_map_*ReActAgentTest#testConfiguredAgentDeclares*,#testBareAgentDeclaresNoBuiltInPrompt,#testUndeclaredPromptsAreNotLookedUp,#testDeclaredPromptThatCannotBeResolvedIsReported; Python counterparts intest_react_agent.pyReActAgentSubagentTest#callerDrivenCallRunsTheReActLoopInsideTheChildScope;test_caller_driven_react_subagentReActAgentSubagentTest#modelDrivenDelegationRoutesToTheDeclaredChildAndClosesTheLoop;test_model_driven_react_subagent_delegationReActAgentSubagentTest#toolCallLoopRunsInsideTheChildScope;test_child_tool_call_loop_runs_inside_the_child_scopeReActAgentSubagentTest#bareChildDoesNotInheritTheParentPrompt,#bareChildDoesNotInheritTheParentOutputSchemaNot verified: cross-language invocation of a ReActAgent child (Java caller with a Python child and vice versa, tracked in #1141); recovery of an in-flight child call (covered by the framework's own recovery tests); model-side schema violations.
API
InternalSubagentResourceDescriptorBuilderfrom [api][plan][runtime] Use generic resource descriptors for sub-agents #1242.inputmap without a prompt; built-in prompts are read only when the agent's own action config declares them.action_config/get_action_config_valueresolve against the active (child) plan inside a sub-agent scope; root-level behavior is unchanged.Documentation
doc-neededdoc-not-neededdoc-includedWas this patch authored or co-authored using generative AI tooling?
Generated-by: Qoder