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

[helloworldtang]

Problem

ReActAgent.extractResponseData rebuilds the final response message when promoting the generate_response tool payload into STRUCTURED_OUTPUT metadata. The rebuild drops the source message's id, timestamp and usage fields: a fresh Msg.builderForRole(...) call synthesizes a new random id and a fresh timestamp, so the persisted final message carries a different identity than the message the tool built, and the usage field is silently lost at this stage.

This is inconsistent with the other message-rebuild paths in the same class — wrapNativeStructuredResult and markRetryResidue — which both preserve id/timestamp/usage.

Fix

Copy id, timestamp and usage from the source message in extractResponseData. Metadata promotion logic is unchanged.

Testing

• New regression test ReActAgentStructuredOutputTest#testToolBasedExtractionPreservesMessageIdentity captures the tool-built response message via a POST_ACTING hook and asserts the final returned message preserves its id and timestamp. It fails on the previous behavior with two distinct message ids.
• ReActAgentStructuredOutputTest fully green (9 tests) with the fix.

Note

The final message's usage field is subsequently owned by mergeCollectedMetadata (aggregated across model calls); the copy here keeps the intermediate rebuild consistent with the other rebuild paths.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[oss-maintainer]

Summary

Aligning extractResponseData with the other rebuild paths so the structured result keeps the source message's identity is the right direction, and builderForRole already preserves name/timestamp elsewhere. COMMENT only: one added line is a no-op on the real path, and the id propagation needs a downstream-uniqueness check.

Findings

• [Warning] ReActAgent.java:1607 — .usage(...) is unconditionally overwritten by mergeCollectedMetadata, which owns the aggregated usage; the added line changes no observable behavior.
• [Warning] ReActAgent.java:1602 — the extracted message now shares its id with the tool result message that carries it; confirm no consumer assumes ids are unique within a context.

Suggestions

Keep .id()/.timestamp(), drop or re-document .usage(), and add the id-collision case (generate_response tool result vs. final structured message) to the test so the propagation contract is pinned down.

---

Automated review by github-manager-bot

[oss-maintainer]

.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.

[oss-maintainer]

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.

[helloworldtang]

感谢评审，两条 findings 都成立，处理如下：

1. .usage(...) 为 no-op — 确认属实：该路径上 mergeCollectedMetadata 随后会用聚合值覆盖 usage 字段（回归测试的注释里也注明了这一点）。保留这行是有意的一致性选择：本 PR 的动机就是让三条消息重建路径（wrapNativeStructuredResult、markRetryResidue、extractResponseData）完全对齐，删掉 .usage() 反而引入例外。已按你的建议在代码注释中写明这一事实。

2. id 的下游唯一性 — 论证如下：response_msg 的 id 由工具内部 Msg.builder() 生成，此前只存在于 tool result 的 metadata 中，从未作为独立消息进入会话上下文；携带它的 tool result 消息的 id 由 buildToolResultMsg 独立生成，两者不同；成功后 compressStructuredOutputContext 会移除 tool result 相关消息，最终 context 中该 id 恰好出现一次。已按建议把该契约钉进测试：断言「preserved id 在最终 context 中恰好出现一次」与「context 内无重复消息 id」。

已推送补充 commit c545181（基于最新 main 重放）。

[oss-maintainer]

Summary

Re-re-review after the new commit test(core): pin id-propagation contract in extractResponseData, document usage copy. Both points from my previous review were taken exactly as suggested, and the fixes are the right ones:

• The .usage(...) line is now documented in place as defensive rather than left as a silent no-op — the comment states plainly that mergeCollectedMetadata overwrites usage with the call's aggregated value on this path, which is the outcome a reader needs to know when touching this method.
• The id-propagation contract is pinned by assertion instead of left as an open question: preserved id must appear exactly once in the conversation context, plus a stronger companion assertion that the context contains no duplicate message ids at all. That second assertion is the valuable one — it makes the whole "ids are unique within a context" invariant executable, so any future rebuild path that leaks a duplicate id fails here rather than in a downstream consumer.

Verified against the head commit: the assertions count over agent.getAgentState().getContext() and compare against the tool-built message's own id, so they exercise the real end-to-end path rather than the extracted object in isolation.

No outstanding concerns. Trivial, well-scoped core fix with a test that outlives the immediate change.

---

Automated review by github-manager-bot