Repository navigation
[api][runtime][java][python] Add event-backed chat calls from actions - #1220
Conversation
Run complete Java ctx.chat jobs against the packaged JDK 21 continuation executor with synchronous and asynchronous model/tool execution. Cover lazy request snapshots, repeated successful and failed awaits, sequential calls, multiple keys, caller-memory injection, private-event isolation, and ordinary event buffering. Use a deterministic counting provider without external services. Validation: four tests passed on each of Flink 2.2 and 2.3 with JDK 21; the two parameterized test methods explicitly skip on JDK 11. Spotless and diff checks passed. RAT recognized the new test's Apache header; the repository-wide scan reported only the pre-existing ignored local .codex/config.toml. Related to apache#1086; supplements PR apache#1220. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
|
Please review this PR when you get a chance @pltbkd. |
sangkyoonnam
left a comment
There was a problem hiding this comment.
Two events with equal attributes from one input record can make overlapping executions of the same action fail at ctx.chat, even with durable execution off (inline, ChatCallManager.java:134). Otherwise FAILED-response replay, the cancellation and persistence error paths, and the gather rejection held up. Local checks passed: 268 runtime tests (23 skipped), 521 Python API and chat-call tests, and 344 Python runtime tests; I didn't run the MiniCluster tests.
I also probed a crash after the tool ran but before the second model turn: recovery gave tool=1, model=2 and one output, because the bootstrap request's id is deterministic and goes into the envelope attributes that child action-state lookup uses. The existing phase 3 stops after model call 2, so a test for that mid-round point seems worth adding.
|
|
||
| private void register(ChatInvocation call) { | ||
| if (calls.putIfAbsent(call.getId(), call) != null) { | ||
| throw new IllegalStateException("Duplicate chat call: " + call.getId()); |
There was a problem hiding this comment.
Two in-flight callers with the same identity collide here. ChatCallOwner derives the id from SubagentIdAllocator (key, sequence number, action name, event type and attributes, but not the event id), and ChatCallManager is operator-wide. In a probe where one action sends new Event("ask") twice and an ask action awaits ctx.chat(...) with two async threads, the second caller fails the task:
java.lang.IllegalStateException: Duplicate chat call: c56f87c4-9c56-3867-98c7-2d38b559b136-0
The probe configures no action-state store, so the older durable-key aliasing isn't what fails here; with a store enabled, identical events also alias through ActionStateUtil.generateUUIDForEvent, which hashes attributes only, and that's a separate recovery issue. Could these overlapping callers get distinct ids that stay replay-stable, with an operator test that runs with persistence off?
There was a problem hiding this comment.
Thanks for catching this. The collision comes from a shared identity problem: internal subagents and chat calls use SubagentIdAllocator, while ActionStateUtil has the related issue tracked in #1084. Neither reliably distinguishes separate event occurrences with identical attributes.
I suggest addressing this through a unified, replay-stable execution identity shared by durable state, internal subagents, and chat calls, rather than adding a chat-specific fix in this PR. The chat case should be included in that work, with regression coverage for overlapping calls when persistence is disabled. cc @pltbkd
Would you be okay with tracking this under #1084 and keeping the fix separate from this PR?
There was a problem hiding this comment.
Sounds good, I'm fine tracking this under #1084. I reran the same probe on a5e6270, and it now fails in the shared registry with the same id: Duplicate subgraph call: c56f87c4-9c56-3867-98c7-2d38b559b136-0. Chat calls and internal subagents both register through SubgraphCallManager now and both still derive their ids through SubagentIdAllocator, so a fix that only covers durable-state keys would leave this persistence-off case open. I can post the chat repro on #1084 so whoever picks it up has that case to test against.
| RunnerContextImpl.checkNoPresetLineage(supplied); | ||
| String id = owner.nextCallId(); | ||
| Optional<Outcome<ChatResponseEvent>> cached = | ||
| context.tryGetCachedResult("builtin-chat:" + id, ChatResponseEvent.class); |
There was a problem hiding this comment.
Within one caller identity, replay matches the next slot by builtin-chat:<id>, and that id carries the await ordinal but no digest of the model, messages or prompt arguments. In a probe that recorded a response for "hello", stopped at a later durableExecuteAsync, changed the prompt to "changed" and restored, the action got answer:hello back with one model call. That follows the stable-id-first contract durable_execute uses, so this is non-blocking and docs-only: chat_models.md:275 could say replay goes by caller identity and await order without comparing request contents, so changing a request at the same position reuses the recorded response.
Run complete Java ctx.chat jobs against the packaged JDK 21 continuation executor with synchronous and asynchronous model/tool execution. Cover lazy request snapshots, repeated successful and failed awaits, sequential calls, multiple keys, caller-memory injection, private-event isolation, and ordinary event buffering. Use a deterministic counting provider without external services. Validation: four tests passed on each of Flink 2.2 and 2.3 with JDK 21; the two parameterized test methods explicitly skip on JDK 11. Spotless and diff checks passed. RAT recognized the new test's Apache header; the repository-wide scan reported only the pre-existing ignored local .codex/config.toml. Related to apache#1086; supplements PR apache#1220. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
6f6a7ee to
ebbb90d
Compare
Run complete Java ctx.chat jobs against the packaged JDK 21 continuation executor with synchronous and asynchronous model/tool execution. Cover lazy request snapshots, repeated successful and failed awaits, sequential calls, multiple keys, caller-memory injection, private-event isolation, and ordinary event buffering. Use a deterministic counting provider without external services. Validation: four tests passed on each of Flink 2.2 and 2.3 with JDK 21; the two parameterized test methods explicitly skip on JDK 11. Spotless and diff checks passed. RAT recognized the new test's Apache header; the repository-wide scan reported only the pre-existing ignored local .codex/config.toml. Related to apache#1086; supplements PR apache#1220. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
ebbb90d to
478fabb
Compare
|
Hi @wenjin272, thanks for this PR. I went through it and found it is using some components of the internal sub-agent. I suppose it's because the root requirement is a common one: dispatch a child event flow, suspend the caller, and resume it on a terminal event. Internal sub-agent also requires isolation, but most of the execution is the same, and both face the same issues like triggering and restoring. I'd suggest a full refactor of the internal sub-agent execution in this PR, separating the execution part from the isolation part so the former can be shared between chat and sub-agent instead of duplicating the implementation and the problem solving; with chat re-implemented on that basis, its own implementation should stay thin enough to keep the overall change size comparable to this PR. I'm willing to help with the development and the review. The determinism question can be discussed separately — the current trade-off matches how internal sub-agent and action state behave today, where two events with identical attributes are not supported yet. What do you think? |
Run complete Java ctx.chat jobs against the packaged JDK 21 continuation executor with synchronous and asynchronous model/tool execution. Cover lazy request snapshots, repeated successful and failed awaits, sequential calls, multiple keys, caller-memory injection, private-event isolation, and ordinary event buffering. Use a deterministic counting provider without external services. Validation: four tests passed on each of Flink 2.2 and 2.3 with JDK 21; the two parameterized test methods explicitly skip on JDK 11. Spotless and diff checks passed. RAT recognized the new test's Apache header; the repository-wide scan reported only the pre-existing ignored local .codex/config.toml. Related to apache#1086; supplements PR apache#1220. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
478fabb to
a5e6270
Compare
|
I've updated this PR so that chat calls and internal subagents share an internal subgraph runtime, rather than maintaining two orchestration paths or implementing chat as a subagent. ArchitectureThe shared layer has three main responsibilities:
What stays in each adapter
Execution and recoveryOn first await, a chat call either reuses its recorded terminal response or registers an invocation and emits its bootstrap. The caller yields, the existing Chat/Tool actions run, and the shared invocation completes after all scheduled actions retire. The chat adapter records the terminal response and resumes the caller. The invocation future and action counters are runtime-only, not checkpointed. Recovery starts from root callers and reconstructs the call tree. Completed child Captured subagent action failures are persisted as Main integration and scopeThis revision is rebased on main, including #1185: No public subgraph DSL or standalone Tool/RAG call API is added. Java awaits still require the JDK 21 continuation runtime; the shared-worker limitation (#1213) and unified caller identity work (#1084) remain separate. Migration of in-flight internal-event state from the old implementation is not supported. Validation on the rebased revision: Java API/Plan/Runtime/MCP on JDK 21 (2,439 passed, 44 skipped), JDK 11 focused regression (173 passed, 20 skipped), Python API/Plan/Runtime (1,217 passed, 15 skipped), and Flink 2.3 MiniCluster tests with rebuilt artifacts (4 Java tests on JDK 21 and 5 Python tests on JDK 11 passed). The PR body includes the contract-to-test mapping and remaining verification gaps. |
pltbkd
left a comment
There was a problem hiding this comment.
Thanks for the update. Overall it looks fine to me. I'm now working on integrating the JDK 21 parallel execution engine with call action execution, including chat and subagent. Some execution-related changes need to happen there, so I've only left a few comments unrelated to execution.
| setAttr(BOOTSTRAP, bootstrap); | ||
| setAttr(DELEGATE_TYPE, delegate.getType()); | ||
| setAttr(DELEGATE_ATTRIBUTES, delegate.getAttributes()); | ||
| setAttr(DELEGATE_ID, delegate.getId().toString()); |
There was a problem hiding this comment.
I suggest leaving the deterministic id strategy for SubgraphEvent and the delegated events unchanged, since we plan to revisit it in #1084. So we don't need to set DELEGATE_ID here, nor compute a deterministic eventId for the delegated event.
There was a problem hiding this comment.
Thanks! Removed DELEGATE_ID from the envelope attributes and restored ordinary event IDs for internal-subagent bootstraps. Chat still derives a stable request ID from the invocation ID because replayed responses retain their original request ID and must pass correlation checks; it is no longer copied into delegate_id for ActionState lookup. Added regression coverage for both behaviors. The unified event-identity work remains in #1084.
| */ | ||
| @Nullable private RunnerContextImpl.MemoryContext isolatedMemoryContext; | ||
|
|
||
| public final class InternalSubagentCallStatus extends SubgraphInvocation<List<Object>> { |
There was a problem hiding this comment.
Better to rename it to InternalSubagentInvocation?
There was a problem hiding this comment.
Thanks, renamed it to InternalSubagentInvocation and updated the related fields, accessors, and Javadocs.
| * <p>Call ID allocation is confined to the action/mailbox thread. | ||
| */ | ||
| public final class ChatCallOwner { | ||
| private final SubagentIdAllocator ids; |
There was a problem hiding this comment.
It's a bit strange to have a SubagentIdAllocator here. Maybe we could simply make it a common component by renaming it to DeterministicIdAllocator, ReplayableIdAllocator, or something similar?
There was a problem hiding this comment.
Thanks, renamed it to DeterministicIdAllocator and moved it to runtime.utils, shared by chat and subagents. The allocation algorithm and hashed field names remain unchanged, so this rename does not change the generated IDs.
Expose lazy action-owned chat calls as DurableFuture handles over the existing built-in chat/tool event loop. Keep invocation tracking in the runtime chat package and group per-action references in ChatContext. Privately route chat events while preserving ordinary event buffering and caller resource/memory scope. Persist terminal ChatResponseEvents through durable action state. Repeated awaits reuse the result; failed responses raise catchable chat errors while cancellation, persistence and event-delivery failures remain runtime failures. Java waits reuse the main branch's resolveAsync path without adding another durable slot. Support Python coroutine calls on JDK 11 and 21. Java awaits require JDK 21 continuations and reject unsupported runtimes before request dispatch. Document same-action use and the shared-pool limitation tracked by apache#1213; low-JDK Java internal calls and parallel chat composition remain out of scope. Place chat-call usage after the event-style example and focus the documentation on user-visible semantics rather than scheduler implementation details. Rebased onto main bd527d7, which includes internal sub-agent support (apache#1138). Related to apache#1086. Validation after rebase: - JDK 21 API/Plan/Runtime and MCP module tests: 2395 passed, 44 skipped. - JDK 21 focused chat/subagent/async/durable tests: 151 passed, 7 skipped. - JDK 11 focused tests: 126 passed, 24 skipped. - Python API/runtime tests: 860 passed, 14 skipped. - Rebuilt Flink 2.3 JARs and Python wheel; MiniCluster chat/subagent tests: 4 passed on JDK 21, and Python chat passed on JDK 11. - Spotless, Ruff, and git diff --check passed. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
Run complete Java ctx.chat jobs against the packaged JDK 21 continuation executor with synchronous and asynchronous model/tool execution. Cover lazy request snapshots, repeated successful and failed awaits, sequential calls, multiple keys, caller-memory injection, private-event isolation, and ordinary event buffering. Use a deterministic counting provider without external services. Validation: four tests passed on each of Flink 2.2 and 2.3 with JDK 21; the two parameterized test methods explicitly skip on JDK 11. Spotless and diff checks passed. RAT recognized the new test's Apache header; the repository-wide scan reported only the pre-existing ignored local .codex/config.toml. Related to apache#1086; supplements PR apache#1220. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
Extract common event routing, invocation lifecycle and root-driven recovery while keeping chat and subagent input/output, resource and memory semantics in their adapters. Persist captured child failures as event outputs and wait for scheduled siblings before completing failed calls. Resolve Python action configuration in the active resource scope. Add arbitrary-subgraph, fan-out, failure-recovery, isolation and nested-chat coverage. Public APIs remain unchanged; legacy in-flight internal-event state is not compatible. Caller identity unification, worker-pool starvation and Java internal calls below JDK 21 remain separate work. Validation: Java runtime 1278 tests (28 skipped), Python runtime 345 passed, clean-built Flink 2.3 Java MiniCluster 4 passed on JDK 21, and Python E2E 5 passed on JDK 11. Spotless, Ruff and diff checks passed. Related to apache#1086. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
Return the complete ChatResult from Java and Python event-backed chat calls. Preserve message blocks and invocation metadata through the future bridge and durable replay. Rebase onto main 7dcbe9f, retaining descriptor-based subagent setup, continuation-native subagent waits, and Python lifecycle registration. Move the chat guide into the reorganized documentation chapter. Address review feedback: rename InternalSubagentInvocation, share runtime.utils.DeterministicIdAllocator, and remove delegate_id from subgraph state identity. Restore ordinary subagent bootstrap event IDs while retaining stable chat request IDs for response correlation on replay. Validation: clean Java compilation; JDK 21 API/Plan/Runtime/MCP tests: 2582 passed, 45 skipped. Python API/Plan/Runtime tests: 1265 passed, 15 skipped. Spotless, Ruff, Hugo build and git diff checks passed. MiniCluster E2E and JDK 11 execution were not rerun for this revision. Related to apache#1086; unified event identity remains tracked by apache#1084. Generated-by: Codex CLI 0.153.4 (GPT-5) Co-authored-by: Codex <codex@openai.com>
a5e6270 to
bc8b4ee
Compare
|
Thanks for the update! LGTM. |
…rop legacy UUID comments Addresses review feedback on apache#1342: - Fix 5 failing ActionExecutionOperatorTest fixtures: seedActionState/ getStoredActionState/testUnsupportedObservationValueIsSkippedWithoutChangingActionSuccess built InputEvents without the root occurrence identity the operator now assigns, so their keys no longer matched the operator's. - Move occurrence-identity generation out of ActionStateStore into a shared path independent of durable storage: the operator now derives a root event's business-key identity directly from its own keyed-state serializer via a new ActionStateKeyEncoder field (rather than delegating through DurableExecutionManager/ActionStateStore, which no-opped when no durable store was configured), and derives an action execution's identity as hash(triggeringEventOccurrenceId, actionIdentity) via a new ActionStateUtil.deriveActionExecutionId, needing neither a key encoder nor a durable store. Removed generateBusinessKeyIdentity/ generateActionExecutionId from the ActionStateStore interface and its Kafka/Fluss implementations accordingly. - Removed historical issue-number/"before this fix" wording from the production-code comments this patch touches. - Kept the ActionStateUtil attributes-only fallback for events without an occurrence identity rather than making it throw: removing it outright would require migrating a large pre-existing lower-level test surface (ActionStateUtilTest, KafkaActionStateStoreTest, FlussActionStateStoreTest, ActionStateKeyEncoderTest) that predates occurrence identity and constructs events without assigning one; left as a follow-up. - Chat/internal-subagent call-path identity (issue apache#1220) intentionally left for a follow-up PR, per reviewer's own suggestion. Generated-by: Claude Code 2.1.316 (Claude Sonnet 5)
Linked issue: #1086
Purpose of change
Actions can use Java
ctx.chat(...).await()or Pythonawait ctx.chat(...)without separate request/response actions. Scope: chat and its tool rounds, not standalone Tool/RAG calls.Runtime flow
ctx.chatsnapshots the request and returns a lazyDurableFuture<ChatResult>.ChatInvocationand queues a privateSubgraphEventbootstrap. The caller suspends while existing Chat/Tool actions run.ActionState; runtime futures are not checkpointed.Key decisions
Behavioral Semantics
Interaction decisions
Behavioral contracts
ChatResult, including message blocks and available invocation metadata; FAILED responses raise a catchable chat exception.gather; cross-action use is unsupported and unchecked. Existing model/tool timeouts apply; no whole-chat timeout is added.Failure behavior
FAILED chat responses raise
ChatResponseEvent.ChatResponseException(Java) orChatResponseError(Python). Missing, duplicate or mismatched responses are protocol errors. Captured subagent action exceptions persist as diagnostic type/message events, reconstructed asIllegalStateExceptionand exposed through the subagent error result. Cancellation, fatal errors, serialization, persistence and dispatch failures propagate. Invalid requests and unsupported runtime/composition raise without fallback.Tests
ChatDurableFutureTest,ChatCallManagerTest, Pythontest_chat_call.pyChatCallTest,SubgraphCallTest, Java/Python MiniCluster chat testsChatCallTest.chatInsideInternalSubagentUsesChildResources, Python nested-chat and internal-subagent E2EChatCallManagerTest,ChatCallTest,InternalSubagentRecoveryTestSubgraphInvocationTest,InternalSubagentRecoveryTest,SubgraphCallTestValidation for revision
bc8b4eeca(rebased onto main7dcbe9f40):git diff --checkpassed.Not verified in this revision: JDK 11 execution, packaged MiniCluster E2E (Java/Python chat suites passed on the prior revision), live-provider/routing/structured-output/media E2E, real-MiniCluster chat recovery (operator-harness coverage only), chat worker-pool deadlock resolution, or state migration.
Implementation invariants and supporting coverage
SubgraphCallManageris a mailbox-confined registry, not a scheduler. Invocation action counts survive yield/resume and retire only after durable output dispatch. Fan-out, empty graphs, nested-call cleanup and failure siblings have focused tests.SubgraphEventisolates routing; its ActionState identity includes the invocation ID, bootstrap flag, and delegate type/attributes, not adelegate_id. Internal-subagent bootstraps use ordinary event IDs. Chat retains a stable request ID to validate recorded responses on replay. Completed callers do not replay consumed bootstraps. Recovery tests cover before dispatch, after child completion, after await, nested trees, partial outputs and captured failures.runtime.utils.DeterministicIdAllocator; its allocation algorithm and hashed field names are unchanged.InternalSubagentInvocationadapts subagent results to the shared invocation lifecycle. Unified identity work ([Bug][runtime] Durable action-state keys collide for distinct events with identical attributes #1084) is not solved here.API
Add Java
RunnerContext.chat(ChatRequestEvent)/chat(String, List<ChatMessage>)and PythonRunnerContext.chat(model, messages, *, prompt_args=None, output_schema=None), returningDurableFuture<ChatResult>. Event APIs, YAML syntax and subagent public APIs are unchanged. Java awaits require JDK 21 continuation support. Captured subagent failures reconstruct diagnostic exceptions; the original exception object is not preserved. Old in-flight internal-event state cannot be migrated.Documentation
doc-neededdoc-not-neededdoc-includedWas this patch authored or co-authored using generative AI tooling?
Generated-by: Codex CLI 0.153.4 (GPT-5)