Repository navigation
[docs] Improve the Sub-Agents guide - #1268
Conversation
531b7dd to
63f1022
Compare
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for expanding the Sub-Agents guide and documenting the call lifecycle, failure handling, and recovery behavior. I left four comments: two on behavioral accuracy and alignment with current main, and two on making the guide more concise and focused on how users work with sub-agents.
| `combine` batches handles into a `SubagentFutures`, and it works the way `gather` | ||
| works for ordinary async futures. `gather` does not accept a `SubagentFuture`, so | ||
| handles batch this way instead. Every handle in the batch is prepared before any is | ||
| executed, so deferred requests all go out up front and remote runs overlap. Results |
There was a problem hiding this comment.
Could we avoid promising concurrent execution for deferred calls here? In both Java and Python, SubagentFutureGroup prepares all handles, then executes and waits for each durable call one by one (the implementation explicitly leaves batching to TODO #926). In the RemoteReviewSetup example above, the POST is inside the callable, so the second request starts only after the first completes. A minimal reproduction produces POST a -> complete a -> POST b -> complete b.
Please describe combine as grouping handles and preserving result order, without implying that it makes deferred requests overlap. The similar overlap claim at the end of Restrictions should also distinguish async setups, which start remote work during submit, from deferred/internal calls.
There was a problem hiding this comment.
Fixed in both places. combine now claims only grouping and result order. I left the one-by-one execution unstated as well, since #926 will change it.
|
|
||
| An action can submit input to a child directly and await its outcome. This path | ||
| does not need an input schema. | ||
| There is no third form. An internal sub-agent cannot be declared in YAML, because |
There was a problem hiding this comment.
This statement is now out of date with the target branch: main has added internal sub-agent references in YAML in commit 762c2c4ae. For example, a parent can declare subagents: [{name: reviewer, agent: child}], where child names another Agent in the YAML document. Both Java and Python loaders support this.
When rebasing, could we replace this restriction with a reference to the YAML guide's “Declaring sub-agents” section and preserve the navigation link recently added on main?
There was a problem hiding this comment.
Fixed in the rebase. The restriction is gone, the Declare section links to Declaring sub-agents, and the internal section states the agent field. Loaded all three YAML forms against the new loaders to confirm.
|
|
||
| #### A workflow agent as the child | ||
|
|
||
| The example child owns a chat model and answers each review request with its |
There was a problem hiding this comment.
Could we streamline the guide and make it easier to scan? A few concrete suggestions:
- Reuse existing examples. “A workflow agent as the child” repeats the connection/model setup and request/response actions already shown in the Chat with Models guide. Could we link to that example and keep only the child registration and input adaptation here—for example, reading
task["code"]for model-driven calls versus the submitted string for caller-driven calls? - Consolidate repeated explanations. The introduction to internal sub-agents already explains that a child is an ordinary Agent made into a sub-agent through registration; the workflow example immediately repeats this. Likewise, “always durable, no sub-agent timeout option” appears in both Overview and Durability and failover. A brief introduction with a link to the detailed section should be enough.
- Use tables for comparisons. Before the external setup examples, a deferred-vs-async table could show when work starts (first await vs. submit), what users implement (
preparevs. submit/status/fetch hooks), and when to choose each (a request returning the result vs. a remote run queried later). This would help readers choose an approach without extracting these differences from several long paragraphs.
There was a problem hiding this comment.
All three done. The child example links to chat#usage-example and keeps only the input adaptation. The toggle and timeout detail now survives only in Durability and failover. The table uses your three rows.
| Waiting for the result then polls the run's status until it reaches a terminal state, | ||
| and fetches it. | ||
|
|
||
| That is the difference from the deferred mode, which sends nothing when the caller |
There was a problem hiding this comment.
Could we focus these explanations more on how users work with sub-agents and less on the framework’s design rationale? For example:
- Instead of explaining “the POST is the publish, and the polling is the subscribe,” describe the observable behavior: submitting starts a remote run, and awaiting polls its status and retrieves the result.
- In “Work with open handles,” focus on awaiting, combining, cancelling, and accessing session/call IDs rather than why
SubagentFutureis a separate type fromAsyncFutureor where it owns durable execution. - In “Invoke a sub-agent,” replace “rides the tool abstraction” with the practical distinction: model-driven results return to the model, while caller-driven results return to the action.
The prepare, durable ID, and reconciler contracts should stay, since users implementing external setups need those details.
There was a problem hiding this comment.
Done, and the async execution ref link went with the type argument. Invoke now opens on where the result goes. The prepare, durable id, and reconciler contracts are unchanged.
sangkyoonnam
left a comment
There was a problem hiding this comment.
A few accuracy points that @wenjin272's review doesn't already cover, checked against the code at 63f1022c in both languages; I ran the Python side where I could.
One more outside the diff: the Java connection in the example using a ReActAgent without a prompt (around line 363) has no endpoint, and OllamaChatModelConnection throws without it (OllamaChatModelConnection.java:101-104). Python's base_url has a default.
| a child `Agent`. An external sub-agent registers a descriptor that names your setup | ||
| class, and a YAML file can declare it. | ||
|
|
||
| ### Internal: compile a child agent |
There was a problem hiding this comment.
yaml.md on main links to subagents#declare-an-internal-sub-agent (line 512) and #declare-an-external-sub-agent (line 537). Both IDs go away in this rewrite, here and at line 454, so neither link reaches its section. Preserving these IDs or updating the incoming links would fit into the same rebase as the YAML change @wenjin272 asked for.
There was a problem hiding this comment.
Fixed with explicit heading ids, so yaml.md needs no change. Added a check that resolves every inbound ref to this page against its real headings; it reports 5 links, 0 broken.
| - It runs against memory isolated from the caller's, so it never reads what the | ||
| caller persisted. The child's own actions share that memory within one call. | ||
| Isolation is per call. Continuing a session does not carry memory across calls. |
There was a problem hiding this comment.
Only sensory and short-term memory are per call (MemoryContext.createChildContext(), RunnerContextImpl.java:142-145). Long-term memory is the operator's one instance in both languages (RunnerContextImpl.java:776-778, flink_runner_context.py:958), so it's outside this per-call isolation.
| - It runs against memory isolated from the caller's, so it never reads what the | |
| caller persisted. The child's own actions share that memory within one call. | |
| Isolation is per call. Continuing a session does not carry memory across calls. | |
| - Its sensory and short-term memory start empty for each call, including calls | |
| continuing a session, and are shared by the child's actions within that call. | |
| Long-term memory, when configured, is shared with the caller. |
There was a problem hiding this comment.
Fixed with your wording.
| if found is None: | ||
| raise RuntimeError("the review service has no record of this call") |
There was a problem hiding this comment.
A failure raised here isn't folded into a result: DeferredSubagentFuture.execute lets it propagate, so for a call made from an action, recovery fails the action instead of handing the caller success == false. If "no record" is meant as a caller-visible failure, returning an error matches How failures surface; the same applies to the Java tab at line 541 and the comments at lines 516 and 534. If failing the action is the intent, a sentence saying so would help.
| if found is None: | |
| raise RuntimeError("the review service has no record of this call") | |
| if found is None: | |
| return SubagentResult.error("the review service has no record of this call") |
There was a problem hiding this comment.
Fixed in these tabs. The comments now say raising here fails the action, which is what How failures surface already claimed, so the example and that section no longer disagree.
| | `result` | `getResult()` | The answer on success. For an internal sub-agent, the list of `OutputEvent` payloads in emission order. For an external one, whatever the setup produces, such as a string or an object | | ||
| | `error_message` | `getErrorMessage()` | The reason on failure | | ||
|
|
||
| A result is persisted and re-bound after a failover, so it must be JSON-compatible. |
There was a problem hiding this comment.
Python persists durable results with cloudpickle (flink_runner_context.py:1209, 1461), so a Python replay restores the payload's type rather than turning it into a dict; Java does persist through Jackson. The string-key and finite-number checks run on model-facing results (normalize_agent_result, ToolResultUtils.normalizeAgentResult), not at persistence. Splitting the Java and Python replay behavior, removing the Python dict comment at line 1190, and scoping the JSON rule to model-facing results would cover it.
There was a problem hiding this comment.
Fixed. Replay is split per language, and the JSON rule is scoped to model-facing results and tied to normalization rather than to persistence.
| SubagentFuture first = reviewer.submit(ctx, "review the diff"); | ||
| SubagentResult firstResult = first.await(); | ||
|
|
||
| SubagentFuture second = reviewer.submit(ctx, "now the tests", first.getSessionId()); |
There was a problem hiding this comment.
second is never resolved, so by Cancel and resolve this action fails with "finished without resolving". Adding SubagentResult secondResult = second.await(); here and second_result = await second in the Python tab resolves both calls before the action returns.
There was a problem hiding this comment.
Fixed in both tabs. Swept the other examples: fan-out resolves through combine, cancel through cancel().
The Sub-Agents page explained how to declare a sub-agent and stopped there, so a reader could not work out how a call behaves once it is running. Rewrite it around what a user has to know to run one in a job, and keep every statement to public API and observable behavior. Declare covers both forms of a sub-agent and the two kinds of child. An internal child is an ordinary agent, so the page says that nothing in its own code marks it as a sub-agent and shows a workflow agent and a ReActAgent as children. An external sub-agent is a setup class, and the page separates the two execution modes by the criterion a reader can observe: whether one request returns the result, or the remote hands back a run to query later. Invoke covers model-driven and caller-driven delegation, and both end in the same submit call, so the handle is defined once where the reader first meets it. A new section covers what an open handle can do: batching, continuing a session, and cancelling, each with an example. Outcome covers what a call returns, how a failure reaches a caller versus a model, what a replay reuses rather than repeats, and what is reported under the subagent scope. Restrictions collects the limits that used to be scattered or absent. Every example was run against the framework, and the page was checked against the implementation for the claims that have no test of their own: both YAML registration forms load, a setup built from a descriptor reads its configuration back as fields, and the async poll interval defaults to 500. Place the page after the ReAct Agent introduction, since it uses ReActAgent as one of its examples, by swapping the two navigation weights. Generated-by: Qoder (Qwen 3.8-Max) Co-Authored-By: Qoder <noreply@qoder.com> AI-Contributed/Feature: 1323/1329 AI-Contributed/UT: 0/0
159bf9e to
53a55ee
Compare
Correct six statements that did not match the implementation. combine does not make deferred requests overlap, because SubagentFutureGroup executes the prepared calls one by one until apache#926 lands, so the sentence that invited that reading is gone and the parallel-dispatch restriction now separates async setups, which start remote work at submit, from deferred and internal calls. Only sensory and short-term memory are per call, while long-term memory is shared with the caller. A reconciler that raises fails the action instead of reaching the caller, so both examples return an error result. Replay is stated per language, since Python persists with cloudpickle and restores the payload's type while Java re-binds through JSON, and the JSON-compatibility rule is scoped to results a model observes. The Java Ollama connection now carries the endpoint that OllamaChatModelConnection requires. YAML declares internal sub-agents as of 762c2c4, so the restriction saying otherwise is replaced by a pointer to the YAML guide. Both section headings keep explicit ids so the links that guide added still resolve. Two examples did not run as shown. The session-continuation example left its second handle unresolved, which its own Cancel and resolve rule rejects, and the prompted ReActAgent examples named a connection they never registered. Streamline the guide. The workflow child links to the Chat with Models example and keeps only the input adaptation a sub-agent adds. A table separates the two external execution modes by when work starts, what the reader implements, and which remote shape each fits. The durable-execution detail appears once. The invocation and handle sections drop the framework design rationale and keep the prepare, durable id, and reconciler contracts. Generated-by: Qoder (Qwen 3.8-Max) AI-Contributed/Feature: 0/251 AI-Contributed/UT: 0/0
53a55ee to
6c26529
Compare
|
Checked |
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for addressing the previous feedback! The comparison table and revised explanations make the guide easier to follow. I found two remaining example/behavior issues and left two additional suggestions for the section titles and registration flow.
| diff = task["code"] if isinstance(task, dict) else str(task) | ||
| ctx.send_event( | ||
| ChatRequestEvent( | ||
| model="review_model", |
There was a problem hiding this comment.
The referenced Chat with Models example registers ollama_chat_model in Python and ollamaChatModel in Java, but these replacement input actions request review_model / reviewModel. Since this revision removes the declarations of those resources, following the linked example and replacing its input action will fail to resolve the model.
Could we use the resource names from the referenced example in both snippets?
There was a problem hiding this comment.
Fixed. Both snippets now use ollama_chat_model and ollamaChatModel, matching the example they link to.
| before awaiting any of them. An async sub-agent starts its remote run during that | ||
| submit, so those runs overlap. A deferred or internal call does its work while it | ||
| is awaited, so it does not overlap this way, and batching handles yourself with | ||
| [combine](#fan-out-to-several-sub-agents) does not change that. Results still land |
There was a problem hiding this comment.
The statement that combine does not change this behavior is too broad for internal sub-agents. SubagentFutureGroup prepares all handles before waiting, and InternalSubagentSetup.prepare sends the child's bootstrap event. Combining two internal calls therefore bootstraps both children before awaiting either, allowing their work to overlap.
Could we distinguish this from external deferred setups whose actual request runs inside the callable? The model-driven submit-then-await path and explicitly combining internal handles do not have the same behavior.
There was a problem hiding this comment.
Fixed. The distinction now lives in the combine section, and the restriction points there instead of asserting that combine changes nothing.
| yields the result. The same two calls drive `remote_reviewer` and `remote_runner` | ||
| unchanged. Only the registration differs. | ||
|
|
||
| ## Work with open handles |
There was a problem hiding this comment.
Could we make these section titles more explicit?
- Rename “Work with open handles” to “Manage sub-agent calls”. “Open handles” is abstract and can suggest file or resource handles; the section actually covers grouping calls, continuing sessions, and cancellation.
- Rename “The outcome of a call” to “Results and failures”, and move durability/recovery and monitoring into separate sections. This makes it clearer where to find return values and error handling, while giving recovery and observability their own headings.
There was a problem hiding this comment.
Done. Manage sub-agent calls, and Results and failures with Durability and failover and Metrics and Trace Log promoted to their own sections.
| Registering the child takes one of two forms. You register a descriptor, or you | ||
| register the `Agent` itself. Which one you need follows from who calls it. A chat | ||
| model can delegate only to a sub-agent it can describe and build arguments for. So a | ||
| child exposed to a model needs the descriptor form, and that descriptor carries a |
There was a problem hiding this comment.
Could we organize the internal sub-agent registration section in this order?
First, briefly define the two calling modes: model-driven calls let the model choose when to delegate and construct the arguments, with results returned to the model; caller-driven calls let an action explicitly submit input and receive the result. The invocation examples can stay in the later Invoke section.
Then summarize the registration requirements in a table:
| Parameter | Model-driven calls | Caller-driven calls |
|---|---|---|
agent |
Required | Required |
description |
Optional, recommended | Optional |
input_type / input_schema |
Exactly one required | Optional; do not set both |
Follow the table with descriptor-based registration, then the shorter direct registration form (parent.add_resource(..., ReviewAgent()) / parent.addResource(..., new ReviewAgent())) for action-only use, where neither description nor input declaration is needed. The YAML reference could follow these examples.
This would explain the terminology before using it in the table, help readers choose a registration form, and avoid implying that description is mandatory.
There was a problem hiding this comment.
Done, with one deviation: the two calling modes are named in a single sentence rather than redefined, because Overview already defines them and a second definition would give two places to keep in step. The table follows, then the descriptor form, the direct form, and the YAML pointer. The description row also caught a contradiction, since the section required a description while the delegate section said a missing one falls back to a generic one. build() requires only agent, so the table is right and that sentence is gone.
Two examples did not work as written. The child's input action named a model setup that this revision no longer declares, so it now uses the resource names of the Chat with Models example it links to. And the claim that batching handles yourself does not change the overlap behavior was too broad: SubagentFutureGroup prepares every handle before running any, and preparing an internal child sends its bootstrap event, so combining two internal calls lets their work overlap. An external deferred setup instead issues its request when the batch runs it. The distinction now sits with combine, and the parallel-dispatch restriction points there rather than repeating a rule that does not hold for both. Reorganize the internal registration section around the choice a reader is making. The two calling modes are named before they are used, and a table gives what each parameter requires per mode. That also corrects a statement implying a model-driven child must carry a description, which build() does not require and which the delegate section already said falls back to a generic one. Rename two sections to say what they cover: Work with open handles becomes Manage sub-agent calls, and The outcome of a call becomes Results and failures. Durability and failover and Metrics and Trace Log move up to their own sections, since neither describes the outcome of a call, and the Actions guide already carries durable execution at the same level. Generated-by: Qoder (Qwen 3.8-Max) Co-Authored-By: Qoder <noreply@qoder.com> AI-Contributed/Feature: 47/47 AI-Contributed/UT: 0/0
Linked issue: Closes #1258
Purpose of change
The Sub-Agents page covered registration and the two ways a call can start, and stopped there. A reader could not work out from it how a call behaves once it is running. This rewrites the page around what a user has to know to run a sub-agent in a job, and keeps every statement to public API and observable behavior.
submitcall.ReActAgentas one of its examples.Tests
javac --release 11against freshly builtapi,plan, andruntime. Clean.AgentPlan.from_agentbuilds with both external descriptor registrations, descriptor arguments bind to the setup's fields,status_poll_interval_millisdefaults to 500, and a deferredpreparereturns a working(id, call, reconciler)triple.api,plan, andruntime.docs/setup_hugo.shpins, builds the site with no errors or warnings. Every{{< ref >}}indocs/contentwas resolved against the target pages' real headings in both directions: this page's outbound refs, and the five inbound links other pages point at it.combinetreats an internal child and an external deferred setup.git diff --checkpassed.API
No public API or runtime behavior changes.
Documentation
doc-neededdoc-not-neededdoc-includedWas this patch authored or co-authored using generative AI tooling?
Generated-by: Qoder (Qwen 3.8-Max)