From 3263259217be4711ed72bc7fd0e76616a506dd78 Mon Sep 17 00:00:00 2001 From: Jackie Date: Tue, 8 Sep 2026 11:33:10 +0800 Subject: [PATCH 1/2] fix(core): preserve id/timestamp/usage when extracting generate_response result extractResponseData rebuilds the final response message while promoting the generate_response payload into STRUCTURED_OUTPUT metadata, but the rebuild drops the source message's id, timestamp and usage fields: a fresh builder synthesizes a new random id and timestamp, so the persisted final message carries a different identity than the message the tool built. Copy the three fields from the source message, consistent with the other message-rebuild paths in this class (wrapNativeStructuredResult, markRetryResidue). Metadata promotion logic is unchanged. Anchored by testToolBasedExtractionPreservesMessageIdentity, which captures the tool-built response message via a POST_ACTING hook and fails on the previous behavior with two distinct message ids. --- .../java/io/agentscope/core/ReActAgent.java | 6 + .../agent/ReActAgentStructuredOutputTest.java | 114 ++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java index 1138ff8042..5210f5f4c8 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java @@ -1580,10 +1580,16 @@ private Msg extractResponseData(Msg responseMsg) { Map metadata = new HashMap<>(responseMsg.getMetadata()); metadata.put(MessageMetadataKeys.STRUCTURED_OUTPUT, responseData); metadata.remove("response"); + // Preserve the source message's identity fields, consistent with the other + // message-rebuild paths (wrapNativeStructuredResult, markRetryResidue): a fresh + // builder synthesizes a new id/timestamp and drops the usage field. return Msg.builderForRole(responseMsg.getRole()) + .id(responseMsg.getId()) .name(responseMsg.getName()) .content(responseMsg.getContent()) .metadata(metadata) + .timestamp(responseMsg.getTimestamp()) + .usage(responseMsg.getUsage()) .build(); } return responseMsg; diff --git a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java index ac1713539c..e1da5d9f15 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java @@ -24,6 +24,7 @@ import io.agentscope.core.agent.test.TestConstants; import io.agentscope.core.hook.Hook; import io.agentscope.core.hook.HookEvent; +import io.agentscope.core.hook.PostActingEvent; import io.agentscope.core.hook.PostReasoningEvent; import io.agentscope.core.memory.InMemoryMemory; import io.agentscope.core.memory.Memory; @@ -31,6 +32,7 @@ import io.agentscope.core.message.MsgRole; import io.agentscope.core.message.TextBlock; import io.agentscope.core.message.ThinkingBlock; +import io.agentscope.core.message.ToolResultBlock; import io.agentscope.core.message.ToolUseBlock; import io.agentscope.core.model.ChatResponse; import io.agentscope.core.model.ChatUsage; @@ -39,6 +41,7 @@ import java.time.Duration; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.RepeatedTest; @@ -157,6 +160,117 @@ void testStructuredOutputToolBased() { assertEquals("Sunny", result.condition); } + @Test + @DisplayName("generate_response extraction preserves the tool-built message identity") + void testToolBasedExtractionPreservesMessageIdentity() { + // extractResponseData promotes the generate_response payload into STRUCTURED_OUTPUT + // metadata by rebuilding the response message. The rebuild must preserve the source + // message's id/timestamp/usage instead of synthesizing fresh ones. + Map toolInput = + Map.of( + "response", + Map.of( + "location", + "San Francisco", + "temperature", + "72°F", + "condition", + "Sunny")); + + AtomicReference originalRef = new AtomicReference<>(); + @SuppressWarnings("deprecation") + Hook captureHook = + new Hook() { + @Override + public Mono onEvent(T event) { + if (event instanceof PostActingEvent post + && post.getToolResultMsg() != null) { + for (ToolResultBlock block : + post.getToolResultMsg() + .getContentBlocks(ToolResultBlock.class)) { + if (block.getMetadata() != null + && block.getMetadata().get("response_msg") + instanceof Msg builtResponseMsg) { + originalRef.set(builtResponseMsg); + } + } + } + return Mono.just(event); + } + }; + + MockModel mockModel = + new MockModel( + msgs -> { + boolean hasToolResults = + msgs.stream().anyMatch(m -> m.getRole() == MsgRole.TOOL); + if (!hasToolResults) { + return List.of( + ChatResponse.builder() + .id("msg_1") + .content( + List.of( + ToolUseBlock.builder() + .id("call_123") + .name("generate_response") + .input(toolInput) + .content( + JsonUtils + .getJsonCodec() + .toJson( + toolInput)) + .build())) + .usage(new ChatUsage(10, 20, 30)) + .build()); + } + return List.of( + ChatResponse.builder() + .id("msg_2") + .content( + List.of( + TextBlock.builder() + .text("Response generated") + .build())) + .usage(new ChatUsage(5, 10, 15)) + .build()); + }); + + ReActAgent agent = + ReActAgent.builder() + .name("weather-agent") + .sysPrompt("You are a weather assistant") + .model(mockModel) + .toolkit(toolkit) + .hook(captureHook) + .build(); + + Msg inputMsg = + Msg.builder() + .name("user") + .role(MsgRole.USER) + .content( + TextBlock.builder() + .text("What's the weather in San Francisco?") + .build()) + .build(); + + Msg responseMsg = agent.call(inputMsg, WeatherResponse.class).block(); + assertNotNull(responseMsg); + + Msg original = originalRef.get(); + assertNotNull(original, "POST_ACTING hook must observe the tool-built response message"); + assertEquals( + original.getId(), + responseMsg.getId(), + "extracted result must preserve the tool-built message id"); + assertEquals( + original.getTimestamp(), + responseMsg.getTimestamp(), + "extracted result must preserve the tool-built message timestamp"); + // Note: the final message's usage field is owned by mergeCollectedMetadata + // (aggregated across model calls), so it is intentionally not asserted here. + } + @Test void testStructuredOutputAutoFallbackToToolBased() { Memory memory = new InMemoryMemory(); From c5451817ae9f4243a3875c706e7d4182d7045f98 Mon Sep 17 00:00:00 2001 From: Jackie Date: Fri, 11 Sep 2026 22:14:49 +0800 Subject: [PATCH 2/2] test(core): pin id-propagation contract in extractResponseData, document usage copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per automated review feedback on #3032: - Assert the preserved message id appears exactly once in the final conversation context and that the context holds no duplicate ids. The source response_msg only ever lived inside the tool result's metadata, so no collision path exists — the test now pins that. - Re-document the .usage(...) copy: on this path mergeCollectedMetadata later overwrites the field with the aggregated value; the copy is kept for consistency with the other message-rebuild paths. --- .../src/main/java/io/agentscope/core/ReActAgent.java | 3 +++ .../core/agent/ReActAgentStructuredOutputTest.java | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java index d9e9258f3d..6c7f3aea58 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java @@ -1598,6 +1598,9 @@ private Msg extractResponseData(Msg responseMsg) { // Preserve the source message's identity fields, consistent with the other // message-rebuild paths (wrapNativeStructuredResult, markRetryResidue): a fresh // builder synthesizes a new id/timestamp and drops the usage field. + // Note: on this path mergeCollectedMetadata later overwrites the usage field with + // the call's aggregated value; the copy here is defensive, keeping the + // intermediate rebuild consistent with the other paths. return Msg.builderForRole(responseMsg.getRole()) .id(responseMsg.getId()) .name(responseMsg.getName()) diff --git a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java index e1da5d9f15..1095926225 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java @@ -269,6 +269,18 @@ public Mono onEvent(T event) { "extracted result must preserve the tool-built message timestamp"); // Note: the final message's usage field is owned by mergeCollectedMetadata // (aggregated across model calls), so it is intentionally not asserted here. + // Pin the id-propagation contract: the source response_msg only ever lived inside + // the tool result's metadata (never as a standalone context message), so the + // preserved id must appear exactly once in the final conversation state. + List contextMsgs = agent.getAgentState().getContext(); + assertEquals( + 1, + contextMsgs.stream().filter(m -> original.getId().equals(m.getId())).count(), + "preserved id must appear exactly once in the conversation context"); + assertEquals( + contextMsgs.size(), + contextMsgs.stream().map(Msg::getId).distinct().count(), + "conversation context must not contain duplicate message ids"); } @Test