Repository navigation
Conversation
eemario
left a comment
There was a problem hiding this comment.
Thanks for working on this! Reusing the existing tool request state makes sense, and the out-of-order test covers the main use case in #1350. I left two inline comments to address before merging, along with two non-blocking test suggestions.
37c3226 to
fc8f266
Compare
|
Thanks @eemario — addressed both blocking items on
Also added the two non-blocking coverage items (cross-language context snapshots + ActionStateSerde restore path for chat context). CI is re-running. |
|
Thanks @HsbcJone for addressing the comments! I’ve verified the fixes and the added tests. Nothing blocking from my side. @wenjin272 , could you take a look when you have a chance? |
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for implementing this and adding the cross-language and recovery tests! I left a few inline comments on the API shape, a remaining array-copy issue, and documentation clarity. The user-context values should follow the existing Event serialization and type-restoration rules.
|
Thanks @wenjin272 — review feedback is addressed on Per-thread mapping:
|
|
Thanks @wenjin272 — addressed your review on
Inline replies are on each thread. CI is re-running on |
7dd8095 to
1124a76
Compare
Optional opaque context on chat, tool, context-retrieval, and decision requests is echoed on the matching response, including chat failure, retry, tool rounds, and model fallback. Callers that omit it are unchanged. Generated-by: Cursor (Grok 4.7) Co-authored-by: mengxiaopeng <602012854@qq.com>
Copy maps and lists, including nested ones, when context is attached so a later change to the caller's object does not rewrite an earlier request. Python keeps context on the built-in request and response events as the supplied JSON, including an object shaped like a PyFlink Row. Generated-by: Cursor (Grok 4.7) Co-authored-by: mengxiaopeng <602012854@qq.com>
Carry application-defined data as a user_context map. Requests take it at construction; responses keep withUserContext so actions can attach it after the existing factories. Snapshotting also copies nested arrays. Generated-by: Cursor (Grok 4.7) Co-authored-by: mengxiaopeng <602012854@qq.com>
Format the request-context sources so Code Style Check matches google-java-format AOSP. Generated-by: Cursor (Grok 4.7) Co-authored-by: mengxiaopeng <602012854@qq.com>
Wrap the ChatModelAction.sendFailure parameters and the ChatRequestEvent constructor arguments to match google-java-format AOSP. Generated-by: Cursor (Grok 4.7) Co-authored-by: mengxiaopeng <602012854@qq.com>
Tool rounds stored user_context as the live object, so a UUID, datetime, Pydantic model, tuple, or Row raised TypeError before ToolRequestEvent was sent. Write the Event JSON form and decode it on read, the same conversion Event.from_json applies, including Row reconstruction. Generated-by: Cursor (Grok 4.7) Co-authored-by: mengxiaopeng <602012854@qq.com>
1124a76 to
a465b44
Compare
|
Follow-up on tip @wenjin272 would you mind another look when you have a moment? |
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for addressing my comments. LGTM
Linked issue: #1350
Closes #1350
Purpose of change
An Action can put optional opaque data on a chat, tool, context-retrieval, or decision request and read that same value on the matching response. The value tells that Action's responses apart. It is not prompt text, tool arguments, or a copy of event attachments.
Runtime flow
The caller sets
context(JavawithContextor the full constructor; Pythoncontext=). The built-in action copies it onto the response it sends.For chat, the action holds the value across retries and, on Java, candidate fallback. A tool round parks it in the tool-request sensory entry and leaves it off the
ToolRequestEvent. The tool response resumes that chat call with the parked value. The terminalChatResponseEvent, success or failure, carries it.Tool, decision, and retrieval actions copy it onto their one response. A retrieval query error throws and sends no response.
Key decisions
contextis optional and untyped, and omitted when null, so an unset request keeps its previous serialized shape.A chat-loop tool request does not inherit the chat context. That value stays in sensory memory and is stamped only on the terminal chat response.
Java uses
withContextbecause anotherObjectargument would clash withoutputSchema. Python takescontexton the constructors and response factories.Behavioral Semantics
Interaction decisions
The value is not written into messages, prompt arguments, tool inputs, the decision request,
request_params, or the vector-store query.Behavioral contracts
Failure behavior
Chat provider errors, exhausted retries, and Java fallback exhaustion send a failed
ChatResponseEventthat still carries context. Decision provider errors do the same. A failed tool call is recorded on theToolResponseEvent, which still carries context. A retrieval query error is thrown and no response is sent. Cancellation still sends no tool or chat response. A value that cannot be serialized fails when the event or sensory entry is written, as with any other attribute.Tests
omittedContextIsAbsentFromTheSerializedEvent,omittedContextStaysOffTheResponse,processToolRequestOmitsContextWhenTheRequestDoes, decision omit case; Pythontest_omitted_context_*,test_tool_response_omits_context_when_the_request_does,test_retrieval_omits_context_when_the_request_doeseachRequestAndResponseRoundTripsItsOwnContext; retrieval, tool, decision, and chat success tests in both languagesprocessToolRequestEchoesContextOnSuccessAndFailureWithoutInjectingIt;queryFailurePropagatesWithoutAResponseAndKeepsTheRequestContexttoolRoundsKeepTheInitialContextAndDoNotCopyItOntoToolRequests;test_tool_rounds_keep_each_request_contextfallbackSuccessAndExhaustionRetainRequestContext;fallbackThenToolRoundRetainsInitialContext; Python retry and tool-round testsdecideargumentsforeignJsonContextSurvivesTheCrossLanguageBoundaryand the Python twin; parked-context JSON in the tool-round testsunknownAttributeIsStillRejectedWhenContextIsPresent;emptyContextIsKeptAndNullClearsIt;test_empty_context_is_keptrestoredRequestStillEchoesContext;test_restored_request_still_echoes_contextJava
mvn -pl api,plan -am testfor these classes plusChatResponseEventTest,DecisionEventTest,BuiltInEventsTest, andCrossLanguageEventSnapshotTest: pass (API 176 run, 14 skipped; plan 156 run). Python pytest of the new tests plus the existing chat, tool, decision, built-in, and snapshot modules: 260 passed, 15 skipped.Not verified: a full Flink job restore. Python has no executable model router, so fallback is covered on Java. Snapshots that omit context are unchanged.
Implementation invariants
The attribute name is
contexton all eight events and in the chat action's tool-request sensory entry. It is stored only when non-null, so an older checkpoint without the key resumes with no context.fromEventallows the key and still rejects every other unknown key. JavaChatModelAction.chat(UUID, String, ...)does not take a context; only the request-event path does. Tool requests created inside a chat loop do not carry the chat context.API
Compatibility impact
Existing constructors and factories keep their signatures. When context is omitted, serialized attributes are unchanged.
withContext,getContext, and the Pythoncontextparameter are additive. YAML event-type aliases are unchanged. A caller who sets nothing new sees the same events as before.Documentation
doc-includedWas this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor (Grok 4.7)