fix(ReActAgent): preserve user chunk callback when using copied toolkit#894
fix(ReActAgent): preserve user chunk callback when using copied toolkit#894JGoP-L wants to merge 4 commits intoagentscope-ai:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a regression where user-configured Toolkit#setChunkCallback(...) was effectively overwritten by ReActAgent during tool execution, ensuring both user chunk callbacks and ActingChunkEvent hook emission work simultaneously (issue #870).
Changes:
- Split tool chunk callback handling into user vs framework-internal callbacks in
ToolExecutor. - Added
Toolkit#setInternalChunkCallback(...)soReActAgentcan emitActingChunkEventwithout overwriting user callbacks. - Preserved user chunk callbacks across
Toolkit.copy()and added a regression test covering the realReActAgentcopied-toolkit scenario.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| agentscope-core/src/main/java/io/agentscope/core/tool/ToolExecutor.java | Separates user/internal chunk callbacks and composes an effective callback for ToolEmitter. |
| agentscope-core/src/main/java/io/agentscope/core/tool/Toolkit.java | Adds internal chunk-callback API and preserves user chunk callback during copy(). |
| agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java | Registers an internal chunk callback (instead of overwriting the user callback) to emit ActingChunkEvent. |
| agentscope-core/src/test/java/io/agentscope/core/tool/ToolEmitterIntegrationTest.java | Adds integration regression test verifying both user callback and ActingChunkEvent receive tool-emitted chunks under ReActAgent. |
| /** | ||
| * Set the framework-internal chunk callback for streaming tool responses. | ||
| * | ||
| * <p>This method is used by ReActAgent to forward tool chunks into ActingChunkEvent hooks | ||
| * without overwriting any user callback configured via {@link #setChunkCallback(BiConsumer)}. | ||
| * | ||
| * @param callback Internal callback to invoke when tools emit chunks via ToolEmitter | ||
| */ | ||
| public void setInternalChunkCallback(BiConsumer<ToolUseBlock, ToolResultBlock> callback) { | ||
| executor.setInternalChunkCallback(callback); | ||
| } |
There was a problem hiding this comment.
Toolkit#setInternalChunkCallback(...) is a public method but appears intended as framework-only API. In this codebase, similar public-but-internal APIs (e.g., Toolkit#callTools(...)) are explicitly labeled in Javadoc as “Internal API - Not recommended for external use.” Consider adding the same explicit warning here to discourage external callers and set expectations about API stability.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
AgentScope-Java Version
1.0.10-SNAPSHOT
Description
Fixes #870.
Previously, when a user configured a chunk callback via
Toolkit#setChunkCallback(...)and then passed that toolkit intoReActAgent.builder().toolkit(toolkit).build(), the callback could be overwritten by the internal callback registered byReActAgentduring tool execution.This PR separates user-defined chunk callbacks from framework-internal chunk callbacks so both can work together:
Toolkit.copy()ActingChunkEventemission inReActAgentToolEmitterIntegrationTestHow to test
mvn -pl agentscope-core -Dtest=ToolEmitterIntegrationTest test