[Tool Call][DSV32] Streamline function call parameters#14750
[Tool Call][DSV32] Streamline function call parameters#14750Fridge003 merged 23 commits intosgl-project:mainfrom
Conversation
Summary of ChangesHello @Muqi1029, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant enhancements to the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the DeepSeekV32Detector to enhance its streaming tool call parsing capabilities for DeepSeek V3.2 models. Key changes include introducing a new _parse_parameters_partially method to handle both JSON and XML parameter formats, and significantly updating the parse_streaming_increment logic to stream tool call arguments incrementally, sending the tool name first and then argument differences. The has_tool_call detection was also improved, and a new update_state method was added for managing parsing state. Corresponding test cases were updated to simulate streaming more accurately by generating chunks based on tokenizer output and expanding XML parameter test coverage. Review comments pointed out the need to correct a return type hint for _parse_parameters_partially, simplify the argument_diff calculation, move a transformers import to the top of the test file, and remove a commented-out line in the tests.
| argument_diff = ( | ||
| func_args_raw[len(self._last_arguments) :] | ||
| if func_args_raw.startswith(self._last_arguments) | ||
| else func_args_raw | ||
| ) |
There was a problem hiding this comment.
This logic for calculating argument_diff can be simplified. Instead of checking startswith and then slicing, you can directly slice and the result will be an empty string if len(self._last_arguments) is greater than or equal to len(func_args_raw), which is a safe and more concise way to get the diff.
argument_diff = func_args_raw[len(self._last_arguments) :]| ), | ||
| ] | ||
| self.detector = DeepSeekV32Detector() | ||
| from transformers import AutoTokenizer |
|
|
||
| # Simulate streaming by splitting into small chunks | ||
| chunks = [text[i : i + 5] for i in range(0, len(text), 5)] | ||
| # chunks = [text[i : i + 5] for i in range(0, len(text), 5)] |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Great! The test was successful. @Muqi1029 |
Thanks for the feedback. The current version only supports streaming at the function-parameter level. Implementing full streaming would require introducing additional logic, which in my view would clutter the codebase (though there may be good tooling out there to help avoid messy conditional logic). Still, I believe this version can be a first step. |
|
OK, do you have any subsequent plans to achieve full streaming? |
Yes, I do have it. But I plan to wait until this version is validated and ready to merge first. |
|
hi, I Have tested ur pr but found an issue missing the arguments |
Really thanks for your testing. Unfortunately, i cannot reproduce your error. I only changed the ip and port, the prompt is completely the same, my result is: And I have requested this prompt several times(>=3), the result is consistently expected. Maybe you introduce other code? |
My test results are the same as yours. Looking forward to full streaming. |
|
@jxz542189 lol, thanks for the endorsement and for helping verify the PR’s correctness. |
|
/tag-and-rerun-ci |
|
I don't know why but I test with this req: And seems this cannot trigger tool on my side... |
|
@JustinTong0323 I know why. One of your previous PR discard system prompt, making tools cannot be rendered in the prompt. This is an urgent bug, I have submitted a PR to fix it: #15092 . And this bug is irrelated about this PR. |
My bad, my test script didn't catch that.... And I could not remember that.. |
|
/rerun-failed-ci |
|
Hi @Muqi1029, I have implemented the parameter streaming version in #15278, and it also passed the test case you added in this PR. Would you kindly review it? If you find my PR acceptable, I will proceed to merge it and close this PR. Nevertheless, I would like to express my gratitude for your contribution! |
Co-authored-by: Muqi Li <muqi1029@gmail.com> Signed-off-by: Xinyuan Tong <xinyuantong.cs@gmail.com>
Signed-off-by: Muqi Li <muqi1029@gmail.com>
db40f83 to
add63f5
Compare
add63f5 to
349ae9e
Compare
|
@JustinTong0323 @Fridge003 You can checkout the test files and then run the test file, you will clearly see the bugs. Can you review this PR again |
I appreciate your efforts and would like to apologize for the previous oversight. I would review this soon. |
|
/tag-and-rerun-ci |
|
/rerun-failed-ci |
|
Thank you truly for your contribution, I will be much more careful next time. |
|
/rerun-failed-ci |
…4750) Signed-off-by: Muqi Li <muqi1029@gmail.com>



Motivation
As titled and Issue #14711.
IMPORTANT: This is an initial version aimed at streamlining function call parameters for
DeepSeek-V3.2. I have tested it, and it passes all tests intest.registered.function_call.test_function_call_parser.TestDeepSeekV32Detector. However, I added quite a few new logics, so I’m not fully confident that there aren’t other bugs. I’m sharing the code here to monitor and track any future bug reports.Update on 2025.12.22:
Now this PR aims to fix bugs about #15278
Checklist