fix(graph): keep streamed tool-call argument fragments that are substrings of accumulated args#4772
Open
logicwu0 wants to merge 1 commit into
Open
fix(graph): keep streamed tool-call argument fragments that are substrings of accumulated args#4772logicwu0 wants to merge 1 commit into
logicwu0 wants to merge 1 commit into
Conversation
1 task
…e substrings of accumulated args
mergeToolArguments() short-circuited on String.contains(): when an incremental
argument delta happened to be a substring of the already-accumulated arguments
(e.g. streaming {"count": 100} digit-by-digit, where the final "0" is contained
in the accumulated "...10"), the fragment was silently dropped, corrupting the
tool-call arguments.
Only a prefix (startsWith) indicates a cumulative-snapshot chunk that should
replace the accumulator; a mere substring does not. Drop the contains() checks
so incremental fragments are always appended. Cumulative-snapshot providers are
still handled via startsWith.
Adds regression tests for the repeated-digit fragment case and a cumulative-
snapshot case alongside the existing split-chunk test.
Closes alibaba#4066
df2af6c to
5c3ae9f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4066
Problem
When a model streams a tool call, the argument JSON arrives split across chunks and
NodeExecutor.mergeToolArguments()reassembles it. It short-circuited onString.contains():An incremental delta can legitimately be a substring of the already-accumulated arguments, and the
contains()branch then drops it. Concrete case: streaming{"count": 100}digit-by-digit as{"count":/1/0/0/}. When merging the accumulated{"count": 10with the final0,previousArguments.contains("0")is true, so the fragment is dropped and the arguments become{"count": 10}.This is the same class of failure as #4066 (
toolInput cannot be null or empty/ lost tool arguments): the merge already handles the name/args split (commit 5c2a0fb), but thecontains()heuristic silently corrupts arguments containing repeated characters.Fix
Only a prefix (
startsWith) indicates a cumulative-snapshot chunk (some providers resend the full arguments each chunk) that should replace the accumulator. A mere substring does not. Removing the twocontains()checks makes incremental fragments always append, while cumulative-snapshot providers are still handled bystartsWith.Tests
Extends
StreamFunctionCallTest(which already covers the split-chunk merge for #4406):testStreamingRepeatedDigitFragmentsShouldNotBeDropped— streams{"count": 100}digit-by-digit; asserts{"count": 100}(fails on pre-fix code: produced{"count": 10}).testStreamingCumulativeSnapshotFragmentsShouldNotBeConcatenated— a provider resending the full arguments each chunk still collapses to a single{"city":"Beijing"}(guards the retainedstartsWithpath).All
spring-ai-alibaba-graph-coreexecutor tests pass;spotless:applyrun.Note on #4066
The original name/arguments split reported in #4066 is already handled by the current
mergeToolCallsimplementation; this PR fixes the remaining argument-corruption edge case and adds regression coverage, so #4066 can be closed.