GitHub user weiqingy deleted a comment on the discussion: Parallel Tool Call Execution
Thanks for working through these. Keeping `tool-call.parallel` as a config knob rather than public API, moving to a separate configurable tool pool, and splitting the `functionId` rename into its own issue all shrink the v1 surface nicely, and I agree with each. On recovery identity: you are right that dropping the per-tool `functionId` keeps matching unchanged and symmetric. `functionId` + `argsDigest` + `currentCallIndex` still catches call-order mismatch at each cursor index, so the unique id is a refinement rather than load-bearing. A separate issue for the "same args, different result" case sounds right. The part I still think needs to be spelled out is the batch recovery state machine. The reserve-in-LLM-order shape is the right target, but today the durable primitives are single-cursor. `appendPendingCall` (`RunnerContextImpl.java:719`) throws unless `currentCallIndex == recoveryCallResults.size()` and does not advance the cursor, so after reserving tool 0 there is no way to reserve tool 1 ahead of it. `finalizeCurrentCall` (`:745`) also finalizes only the current slot. So "reserve N slots up front, merge results back by index/function identity" needs new index-addressable primitives, such as reserve-N and finalize-at-index. This is the same batch-slots-vs-single-cursor issue @joeyutong raised, pinned to the current code. There is also a replay-semantics edge for ordinary tools without reconcilers. `CallResult.pending(...)` stores null/null (`CallResult.java:133`), and the completion-only recovery path reads null/null as a cache hit and returns `Optional.of(null)` (`RunnerContextImpl.java:473-481`). So if a non-reconciler tool is still in flight at failover, a reserved `PENDING` slot could recover as a successful null result instead of re-running. Only the reconciler path has a real `PENDING` handler today. Could the design define what a reserved-but-unfinished slot means on replay for a tool with no reconciler? My assumption would be re-run or record failure, not treat null as the answer. The Python side has the mirror of the same gap. `_DurableAsyncExecutionResult.__await__` (`flink_runner_context.py:157`) submits one call when awaited and yields until it completes, and `_record_call_completion` (`:446`) records with no explicit index. Serially that equals `tool_calls` order, but a parallel batch that records futures as they finish would record completion order, which breaks the "record strictly in tool_calls order" invariant. Since `asyncio.gather` is unsupported in Python actions, Python also needs a real submit-all -> yield-until-all -> ordered-record pass. Could the design spell out this reserve-N / record-in-order / recover-partial flow concretely for both Java and Python? Interlocking with that: I did not see a timeout bounding a batch. `AgentExecutionOptions` has `MAX_RETRIES` / `RETRY_WAIT_INTERVAL` / `NUM_ASYNC_THREADS`, but no per-call or per-batch timeout, and the barrier shape is `allOf(futures) + while (!barrier.isDone()) yield` (`ContinuationActionExecutor.java:132`). The collect-all discussion covers tools that throw, but not a tool that never returns. With an all-or-nothing barrier, one straggler can pin a pool thread indefinitely and hold the N-1 finished results hostage because fan-in only runs after every future completes. Is a per-tool or per-batch timeout in scope for v1, with timeout recorded as per-tool failure so collect-all can proceed, or should "tools must bound their own I/O" be the documented contract next to the idempotency note? Last, lighter point: keeping `durableExecuteAllAsync` off public `RunnerContext` is still the right direction, but the internal seam needs a name. `ToolCallAction` lives in `plan` and reaches durable execution only through the public `RunnerContext` interface (`ToolCallAction.java:100`, `RunnerContext.java:131,:147`). If the batch primitive stays non-public, the built-in action either needs the method on `RunnerContext` anyway, a downcast to a runtime implementation, or batch dispatch has to move below `ToolCallAction` into the runtime/operator layer. Which seam is intended? GitHub link: https://github.com/apache/flink-agents/discussions/855#discussioncomment-17551048 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected]
