Skip to content

fix(core): preserve id/timestamp/usage when extracting generate_response result #3032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
helloworldtang wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix(core): preserve id/timestamp/usage when extracting generate_response result #3032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3263259
fix(core): preserve id/timestamp/usage when extracting generate_respo…
helloworldtang Sep 8, 2026
374ebc7
Merge branch 'main' into fix/extract-response-data-identity
helloworldtang Sep 8, 2026
db5ccfb
Merge branch 'main' into fix/extract-response-data-identity
helloworldtang Sep 9, 2026
e7040ae
Merge branch 'main' into fix/extract-response-data-identity
helloworldtang Sep 10, 2026
5a64548
Merge branch 'main' into fix/extract-response-data-identity
helloworldtang Sep 11, 2026
c545181
test(core): pin id-propagation contract in extractResponseData, docum…
helloworldtang Sep 11, 2026
25b43f0
Merge branch 'main' into fix/extract-response-data-identity
helloworldtang Sep 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,10 +1621,19 @@ private Msg extractResponseData(Msg responseMsg) {
Map<String, Object> 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.
// 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())

@oss-maintainer oss-maintainer Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Id now flows from the tool-built response_msg into the message appended to the session context at line 1381 (scope.state.contextMutable().add(out)), where previously the rebuild synthesized a fresh id. Please confirm no consumer keys on message id uniqueness within a single context: PostActingEvent.getToolResultMsg() (which carries the same response_msg) and the structured result are now id-identical, and anything that de-dupes, reconciles replay, or maps events to messages by id (the AG-UI replay/session-state path is exactly such a consumer) can collapse the two into one. name/timestamp were already copied by mergeCollectedMetadata, so id is the only genuinely new identity being propagated here — worth one sentence in the PR description on which downstream consumers were checked.

.name(responseMsg.getName())
.content(responseMsg.getContent())
.metadata(metadata)
.timestamp(responseMsg.getTimestamp())
.usage(responseMsg.getUsage())

@oss-maintainer oss-maintainer Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.usage(responseMsg.getUsage()) has no observable effect on the only production path. The sole caller of extractStructuredResult(...) (line 1373) is immediately followed by mergeCollectedMetadata(extracted, aggregatedUsage, aggregatedThinking), and that method rebuilds the message with .usage(chatUsage) (line ~1631) using the aggregated usage — so whatever is set here is overwritten. The test comment already concedes this ("usage field is owned by mergeCollectedMetadata"), and the assertion is deliberately omitted. Two options: drop the line and the surrounding rationale for usage, or state in the comment that it is defensive for future callers of extractResponseData that do not go through mergeCollectedMetadata. As written, the title/summary advertises usage preservation that the change does not actually deliver.

.build();
}
return responseMsg;
Expand Down
126 changes: 126 additions & 0 deletions agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentStructuredOutputTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
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;
import io.agentscope.core.message.Msg;
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;
Expand All @@ -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;
Expand Down Expand Up @@ -157,6 +160,129 @@ 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<String, Object> toolInput =
Map.of(
"response",
Map.of(
"location",
"San Francisco",
"temperature",
"72°F",
"condition",
"Sunny"));

AtomicReference<Msg> originalRef = new AtomicReference<>();
@SuppressWarnings("deprecation")
Hook captureHook =
new Hook() {
@Override
public <T extends HookEvent> Mono<T> 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.
// 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<Msg> 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
void testStructuredOutputAutoFallbackToToolBased() {
Memory memory = new InMemoryMemory();
Expand Down
Loading