GH-4586 Fixed some merge exceptions in DeepSeekStreamFunctionCallingHelper
#4598
+221
−5
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.
As mentioned in the issue, there is currently a problem in
DeepSeekStreamFunctionCallingHelper
when merging instances wheretoolCalls()
is an empty list rather than null. This PR fixes that issue.Additionally, I noticed that the class previously lacked unit tests, so I've added relevant test cases.
Through these new tests, I discovered an issue in the
DeepSeekStreamFunctionCallingHelper#merge(ChatCompletionMessage, ChatCompletionMessage)
method:This line has an operator precedence problem. The string concatenation operator
+
has higher precedence than the ternary operator? :
, so the expression is effectively parsed as:This means that if
current.content()
is not null, it is returned directly, and only when it is null does the string concatenation withprevious.content()
occur. This behavior clearly deviates from the intended merge logic.The correct behavior should be to concatenate the contents of both
previous
andcurrent
. This PR fixes the issue, ensuring that message content is properly merged regardless of whethercurrent.content()
is null, and the fix is validated through newly added unit tests.Fixes #4586