Repository navigation
feat(core): 阻止长度截断的不完整工具调用执行并引导模型分块恢复 #2649
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,8 @@ | |
| */ | ||
| public class ReasoningContext { | ||
|
|
||
| private static final String FINISH_REASON_LENGTH = "length"; | ||
|
|
||
| private final String agentName; | ||
| private String messageId; | ||
|
|
||
|
|
@@ -57,6 +59,7 @@ public class ReasoningContext { | |
| private int outputTokens = 0; | ||
| private int cachedTokens = 0; | ||
| private double time = 0; | ||
| private String finishReason; | ||
|
|
||
| public ReasoningContext(String agentName) { | ||
| this.agentName = agentName; | ||
|
|
@@ -78,6 +81,9 @@ public ReasoningContext(String agentName) { | |
| */ | ||
| public List<Msg> processChunk(ChatResponse chunk) { | ||
| this.messageId = chunk.getId(); | ||
| if (chunk.getFinishReason() != null && !chunk.getFinishReason().isBlank()) { | ||
| finishReason = chunk.getFinishReason(); | ||
| } | ||
|
|
||
| // Accumulate ChatUsage | ||
| ChatUsage usage = chunk.getUsage(); | ||
|
|
@@ -159,6 +165,9 @@ public Msg buildFinalMessage() { | |
|
|
||
| // Add all tool calls | ||
| List<ToolUseBlock> toolCalls = toolCallsAcc.buildAllToolCalls(); | ||
| if (FINISH_REASON_LENGTH.equals(finishReason)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The truncation guard keys on the literal finish reason "length", but the check lives in core while other bundled model extensions report length limits with different values: GeminiResponseParser maps candidate.finishMessage() (Gemini's finish reasons are e.g. MAX_TOKENS), ResponsesResponseParser (openai-official) sets finishReason to the response status ("completed"/"incomplete"), and the Anthropic extension never maps stop_reason at all. For those providers a truncated tool call still silently falls back to "{}" arguments and is executed, so the new safety guard only protects OpenAI-chat-completions-style endpoints. Consider normalizing length-limit reasons in the model layer (or matching a set such as {"length", "max_tokens", "MAX_TOKENS"}) so the ToolExecutor guard is provider-agnostic instead of OpenAI-specific. |
||
| toolCalls = markLengthLimitedIncompleteToolCalls(toolCalls); | ||
| } | ||
| blocks.addAll(toolCalls); | ||
|
|
||
| // If no content at all, return null | ||
|
|
@@ -179,6 +188,9 @@ public Msg buildFinalMessage() { | |
| .build(); | ||
| metadata.put(MessageMetadataKeys.CHAT_USAGE, chatUsage); | ||
| } | ||
| if (finishReason != null) { | ||
| metadata.put(MessageMetadataKeys.MODEL_FINISH_REASON, finishReason); | ||
| } | ||
|
|
||
| return AssistantMessage.builder() | ||
| .id(messageId) | ||
|
|
@@ -189,6 +201,42 @@ public Msg buildFinalMessage() { | |
| .build(); | ||
| } | ||
|
|
||
| /** | ||
| * Marks tool calls whose argument stream was truncated when the model reached its output | ||
| * length limit. | ||
| * | ||
| * <p>The accumulator already preserves the legacy fallback of replacing invalid raw argument | ||
| * JSON with {@code {}}. That fallback must remain available for providers that do not report a | ||
| * finish reason, because invalid JSON alone does not prove that the response was truncated. | ||
| * A {@code length} finish reason is the additional evidence that lets the executor distinguish | ||
| * an incomplete response from a legacy malformed payload and safely avoid invoking the tool. | ||
| * | ||
| * <p>Only the affected calls are copied with the internal marker. Complete calls in the same | ||
| * response keep their original instance and remain executable. | ||
| */ | ||
| private List<ToolUseBlock> markLengthLimitedIncompleteToolCalls(List<ToolUseBlock> toolCalls) { | ||
| return toolCalls.stream() | ||
| .map( | ||
| toolCall -> { | ||
| if (!Boolean.TRUE.equals( | ||
| toolCall.getMetadata() | ||
| .get(ToolUseBlock.METADATA_RAW_CONTENT_INCOMPLETE))) { | ||
| return toolCall; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test gap: the early-return branch (a complete tool call inside a finish_reason=length response must stay executable) has no coverage, nor does a mixed response containing one complete and one truncated call in the same response. These pin the most subtle guarantee of this change (only the affected call is blocked; complete calls in the same response still run) and are likely among the 4 uncovered lines Codecov reported. A small ReasoningContextTest case with one valid-args ToolUseBlock plus one truncated block under finishReason("length") would cover both.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test gap: the early-return branch (a complete tool call inside a finish_reason=length response must stay executable) has no coverage, nor does a mixed response containing one complete and one truncated call in the same response. These pin the most subtle guarantee of this change (only the affected call is blocked; complete calls in the same response still run) and are likely among the 4 uncovered lines Codecov reported. A small ReasoningContextTest case with one valid-args ToolUseBlock plus one truncated block under finishReason("length") would cover both. |
||
| } | ||
| Map<String, Object> metadata = new HashMap<>(toolCall.getMetadata()); | ||
| metadata.put(ToolUseBlock.METADATA_OUTPUT_LENGTH_LIMIT, true); | ||
| return ToolUseBlock.builder() | ||
| .id(toolCall.getId()) | ||
| .name(toolCall.getName()) | ||
| .input(toolCall.getInput()) | ||
| .content(toolCall.getContent()) | ||
| .metadata(metadata) | ||
| .state(toolCall.getState()) | ||
| .build(); | ||
| }) | ||
| .toList(); | ||
| } | ||
|
|
||
| /** | ||
| * Build a chunk message from a content block. | ||
| * @hidden | ||
|
|
@@ -297,4 +345,11 @@ public ChatUsage getChatUsage() { | |
| } | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the last non-blank finish reason reported during the current model response. | ||
| */ | ||
| public String getFinishReason() { | ||
| return finishReason; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,28 @@ Mono<ToolResultBlock> execute(ToolCallParam param) { | |
| */ | ||
| private Mono<ToolResultBlock> executeCore(ToolCallParam param) { | ||
| ToolUseBlock toolCall = param.getToolUseBlock(); | ||
|
|
||
| // A model may emit a syntactically partial tool call before reporting "length". The | ||
| // accumulator intentionally keeps the historical "{}" fallback for compatibility, so | ||
| // schema validation alone cannot distinguish this case from a genuine empty argument | ||
| // object. The paired metadata markers provide that evidence and ensure that no tool with | ||
| // potentially destructive side effects is invoked from a truncated argument stream. The | ||
| // error result is returned through the normal execution path, which preserves the tool | ||
| // call ID and prevents the ReAct state from retaining an unmatched pending tool call. | ||
| if (Boolean.TRUE.equals( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rebase note: this diff is based on the old single-arg executeCore(ToolCallParam); current main has refactored it to executeCore(param, requestConfig, internalChunkCallback) with resolveTool() and an external-tool suspension short-circuit (ToolResultBlock.suspended on main), and the branch currently conflicts with main. When rebasing, keep this guard at the very top of the new executeCore so a length-truncated external/HITL tool call returns the error result instead of being suspended with "{}" arguments.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rebase note: this diff is based on the old single-arg executeCore(ToolCallParam); current main has refactored it to executeCore(param, requestConfig, internalChunkCallback) with resolveTool() and an external-tool suspension short-circuit (ToolResultBlock.suspended on main), and the branch currently conflicts with main. When rebasing, keep this guard at the very top of the new executeCore so a length-truncated external/HITL tool call returns the error result instead of being suspended with "{}" arguments. |
||
| toolCall.getMetadata().get(ToolUseBlock.METADATA_OUTPUT_LENGTH_LIMIT))) { | ||
| String errorMsg = | ||
| "Tool arguments were incomplete because the model output reached its length" | ||
| + " limit. The tool was not executed. Do not retry this call unchanged." | ||
| + " Reduce the argument payload and complete the work with multiple" | ||
| + " smaller calls or an available incremental update tool."; | ||
| logger.warn( | ||
| "Blocked incomplete length-limited tool call: name={}, id={}", | ||
| toolCall.getName(), | ||
| toolCall.getId()); | ||
| return Mono.just(ToolResultBlock.error(errorMsg)); | ||
| } | ||
|
|
||
| AgentTool tool = toolRegistry.getTool(toolCall.getName()); | ||
|
|
||
| if (tool == null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The truncation guard keys on the literal finish reason "length", but the check lives in core while other bundled model extensions report length limits with different values: GeminiResponseParser maps candidate.finishMessage() (Gemini's finish reasons are e.g. MAX_TOKENS), ResponsesResponseParser (openai-official) sets finishReason to the response status ("completed"/"incomplete"), and the Anthropic extension never maps stop_reason at all. For those providers a truncated tool call still silently falls back to "{}" arguments and is executed, so the new safety guard only protects OpenAI-chat-completions-style endpoints. Consider normalizing length-limit reasons in the model layer (or matching a set such as {"length", "max_tokens", "MAX_TOKENS"}) so the ToolExecutor guard is provider-agnostic instead of OpenAI-specific.