HsbcJone commented on code in PR #1352:
URL: https://github.com/apache/flink-agents/pull/1352#discussion_r4488702995


##########
docs/content/docs/development/create-agent/events.md:
##########
@@ -278,3 +278,53 @@ Event that your Actions can handle to continue the 
workflow:
 | Call tools | `ToolRequestEvent`, `ToolResponseEvent` | [Tool Use]({{< ref 
"docs/development/calling-llms/tool_use#built-in-events-and-actions" >}}) |
 | Retrieve context | `ContextRetrievalRequestEvent`, 
`ContextRetrievalResponseEvent` | [Vector Stores]({{< ref 
"docs/development/vector-search/vector_stores#built-in-events-and-actions" >}}) 
|
 | Call a decision model | `DecisionRequestEvent`, `DecisionResponseEvent` | 
[Decision Models]({{< ref "docs/development/decision_models#event-driven-calls" 
>}}) |
+
+### Request context
+
+Chat, tool, context-retrieval, and decision request events accept an optional 
`context`
+value: a map or other JSON-compatible object your Action uses to tell 
responses apart.
+The built-in Action copies that value onto the matching response and does not 
add it to
+the prompt, tool arguments, or vector-store query. Omitting it leaves the 
response
+without a `context` attribute, which is what existing callers do.

Review Comment:
   Done in `e963cca`: opening leads with carrying application-defined data; 
removed “existing callers” / attachments comparison.



##########
api/src/main/java/org/apache/flink/agents/api/event/UserDefinedContext.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api.event;
+
+import org.apache.flink.agents.api.Event;
+
+import javax.annotation.Nullable;
+
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Optional opaque application data carried on a built-in request and echoed 
on its response.
+ *
+ * <p>The value is not interpreted, not injected into prompts or tool 
arguments, and omitted
+ * entirely when absent so existing events keep their serialized shape.
+ *
+ * <p>Maps and lists, including nested ones, are copied when attached. A later 
change to the
+ * caller's object does not change an event that already holds the value. 
Other values are stored as
+ * given.
+ */
+final class UserDefinedContext {
+
+    static final String ATTRIBUTE = "context";
+
+    private UserDefinedContext() {}
+
+    static void set(Event event, @Nullable Object context) {
+        if (context == null) {
+            event.getAttributes().remove(ATTRIBUTE);
+        } else {
+            event.setAttr(ATTRIBUTE, snapshot(context));
+        }
+    }
+
+    /**
+     * Returns a detached copy of {@code value}. Maps and lists are copied 
recursively so the event
+     * does not share the caller's mutable structure.
+     */
+    static Object snapshot(Object value) {
+        if (value instanceof Map) {
+            Map<?, ?> map = (Map<?, ?>) value;
+            Map<Object, Object> copy = new LinkedHashMap<>();
+            for (Map.Entry<?, ?> entry : map.entrySet()) {
+                copy.put(snapshot(entry.getKey()), snapshot(entry.getValue()));
+            }
+            return copy;
+        }
+        if (value instanceof List) {
+            List<?> list = (List<?>) value;
+            List<Object> copy = new ArrayList<>(list.size());
+            for (Object item : list) {
+                copy.add(snapshot(item));
+            }
+            return copy;
+        }
+        return value;

Review Comment:
   Thanks @wenjin272 — fixed on `e963cca` (spotless clean-up on `fc43b70`).
   
   `UserDefinedContext.snapshot` now deep-copies arrays (including nested ones) 
the same way it already copied maps and lists, so a later mutation of a 
caller's `String[]` no longer leaks into an event that already holds the value. 
Added coverage for that case in the Java request-context tests.



##########
api/src/main/java/org/apache/flink/agents/api/event/ChatRequestEvent.java:
##########
@@ -54,20 +56,31 @@ public class ChatRequestEvent extends Event {
                             ChatMessage.class,
                             Map.class),
                     BuiltInAttribute.optional("prompt_args", Map.class),
-                    BuiltInAttribute.optionalUntyped("output_schema"));
+                    BuiltInAttribute.optionalUntyped("output_schema"),
+                    
BuiltInAttribute.optionalUntyped(UserDefinedContext.ATTRIBUTE));
 
     public ChatRequestEvent(
             String model,
             List<ChatMessage> messages,
             @Nullable Map<String, Object> promptArgs,
-            @Nullable Object outputSchema) {
+            @Nullable Object outputSchema,
+            @Nullable Object context) {

Review Comment:
   Done on `e963cca`. The field is now `Map<String, Object> userContext` in 
Java and `dict[str, Any] | None` / `user_context` in Python across the built-in 
request and response events.



##########
api/src/main/java/org/apache/flink/agents/api/event/ChatRequestEvent.java:
##########
@@ -145,4 +158,20 @@ public Map<String, Object> getPromptArgs() {
         Map<String, Object> args = (Map<String, Object>) 
getAttr("prompt_args");
         return args != null ? args : Collections.emptyMap();
     }
+
+    /**
+     * Attaches opaque application data to echo unchanged on the matching 
{@link ChatResponseEvent}.
+     * The value is not added to messages or prompt arguments. {@code null} 
omits it.
+     */
+    public ChatRequestEvent withContext(@Nullable Object context) {

Review Comment:
   Done on `e963cca`. Request events take `userContext` / `user_context` at 
construction time only; `withContext()` is gone from the four request events. 
Response events still expose `withUserContext()` for the propagation path.



##########
docs/content/docs/development/calling-llms/chat.md:
##########
@@ -107,6 +107,8 @@ InputEvent → request Action → ChatRequestEvent
 
 A `ChatRequestEvent` can name a model router instead of a chat model. The 
built-in action selects the model first and emits a `ModelRoutingEvent`; see 
[Model Routing]({{< ref "docs/development/calling-llms/model_routing" >}}).
 
+Pass optional `context` (Python) or `withContext` (Java) when one Action sends 
several chat requests and the response Action needs to tell them apart. The 
value is returned unchanged on the `ChatResponseEvent`, including after 
tool-call rounds, retries, model fallback, and a failed response. It is not 
added to the prompt. See [Request context]({{< ref 
"docs/development/create-agent/events#request-context" >}}).

Review Comment:
   Updated on `e963cca`. The chat docs now describe `user_context` as carrying 
application-defined data from a request to its corresponding response, 
including when requests come from one Action or from several.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to