fix(core): preserve failed summary termination state - #2757
Conversation
Keep MAX_ITERATIONS as the generate reason when final summary generation fails, and expose the failure through message metadata for downstream callers.\n\nFixes agentscope-ai#2754
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR adds metadata markers to the summary-error fallback path in ReActAgent.handleSummaryError(). Specifically, it sets SUMMARY_FAILED=true in the message metadata and explicitly assigns GenerateReason.MAX_ITERATIONS to the error fallback message. This brings the error path into symmetry with the successful summary path (which already sets MAX_ITERATIONS via withGenerateReason), while the new SUMMARY_FAILED flag allows callers to distinguish a failed summary from a successful one. The change is minimal, well-targeted, and includes a focused unit test that validates all three assertions (generate reason, metadata flag, and error message text). No bugs or correctness issues found.
| + " generating summary: %s", | ||
| maxIters, error.getMessage())) | ||
| .build()) | ||
| .metadata(Map.of(MessageMetadataKeys.SUMMARY_FAILED, true)) |
There was a problem hiding this comment.
[nitpick] The .metadata(Map.of(...)) call creates an unmodifiable map. The subsequent .generateReason() call correctly wraps it into a HashMap (via the instanceof HashMap check in Msg.Builder.generateReason()), so the current code works correctly. However, this creates an implicit ordering dependency: if a future refactoring moves .metadata() after .generateReason(), the metadata() call would silently overwrite the generate-reason entry. Consider adding a brief comment like // .metadata() must precede .generateReason() or using new HashMap<>(Map.of(...)) directly to make the intent explicit and prevent accidental breakage.
AgentScope-Java Version
2.0.3-SNAPSHOT
Description
When the ReAct loop reaches
maxItersand final summary generation also fails, the fallback message currently uses the defaultMODEL_STOPreason. Downstream callers cannot distinguish this failure from a normal model completion.This change:
GenerateReason.MAX_ITERATIONSon the summary fallback message_summary_failed=truemetadata for explicit failure detectionFixes #2754
Testing
mvn -pl agentscope-core clean test: 2276 tests passed, 9 skippedChecklist
mvn spotless:applymvn test)