Skip to content

Conversation

@fgeygfe
Copy link

@fgeygfe fgeygfe commented Dec 5, 2025

No description provided.

Copilot AI review requested due to automatic review settings December 5, 2025 18:18
@paddle-bot
Copy link

paddle-bot bot commented Dec 5, 2025

Thanks for your contribution!

@paddle-bot paddle-bot bot added the contributor External developers label Dec 5, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds comprehensive support for DeepSeek models (V3.1, V3-0324, and R1) to FastDeploy by implementing both Reasoning Parser and Tool Parser functionality. The implementation follows existing patterns in the codebase and includes thorough test coverage.

Key Changes:

  • Implements DeepSeekReasoningParser to extract reasoning content from DeepSeek model outputs with support for both streaming and non-streaming modes
  • Implements DeepSeekToolParser to parse tool calls from DeepSeek models, supporting two different formats (V3.1 and V3-0324/R1)
  • Adds comprehensive test suites for both parsers covering edge cases and protocol violations
  • Updates documentation in both English and Chinese with examples and error handling guidance

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
fastdeploy/reasoning/deepseek_reasoning_parser.py New reasoning parser implementation for DeepSeek models with <think> tag extraction
fastdeploy/entrypoints/openai/tool_parsers/deepseek_tool_parser.py New tool parser supporting V3.1 and V3-0324/R1 format variations with streaming support
tests/reasoning/test_reasoning_parser.py Adds comprehensive test cases for DeepSeek reasoning parser with 18 test methods
tests/entrypoints/openai/tool_parsers/test_deepseek_tool_parser.py New test file with 15 test cases covering both model formats and edge cases
docs/zh/features/tool_calling.md Chinese documentation with DeepSeek model table and error handling examples
docs/zh/features/reasoning_output.md Chinese documentation for reasoning output with model compatibility table
docs/features/tool_calling.md English documentation with DeepSeek model table and error handling examples
docs/features/reasoning_output.md English documentation for reasoning output with model compatibility table
Comments suppressed due to low confidence (3)

fastdeploy/entrypoints/openai/tool_parsers/deepseek_tool_parser.py:164

  • Except block directly handles BaseException.
                    except:

fastdeploy/entrypoints/openai/tool_parsers/deepseek_tool_parser.py:294

  • Except block directly handles BaseException.
                                except:

fastdeploy/entrypoints/openai/tool_parsers/deepseek_tool_parser.py:322

  • Except block directly handles BaseException.
                        except:

if tool_calls_begin_pos > 0:
# 检查中间是否有非空白字符
between_text = after_reasoning[:tool_calls_begin_pos]
if between_text.strip() and not between_text.strip().isspace():
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic between_text.strip() and not between_text.strip().isspace() is redundant. After calling .strip(), the result is already stripped of whitespace, so .strip().isspace() will always return False for any non-empty string. Simply use:

if between_text.strip():
    # 有非空白字符,协议不规范,不解析工具调用
    return ExtractedToolCallInformation(tools_called=False, tool_calls=None, content=model_output)

This achieves the same result more efficiently.

Suggested change
if between_text.strip() and not between_text.strip().isspace():
if between_text.strip():

Copilot uses AI. Check for mistakes.

def random_tool_call_id() -> str:
"""Generate a random tool call ID"""
return f"chatcmpl-tool-{str(uuid.uuid4().hex)}"
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify the redundant str() conversion. uuid.uuid4().hex already returns a string, so wrapping it in str() is unnecessary:

return f"chatcmpl-tool-{uuid.uuid4().hex}"
Suggested change
return f"chatcmpl-tool-{str(uuid.uuid4().hex)}"
return f"chatcmpl-tool-{uuid.uuid4().hex}"

Copilot uses AI. Check for mistakes.
Comment on lines +212 to +214
self.current_tool_id = (
max(self.current_tool_id, 0) if self.current_tool_id == -1 else self.current_tool_id + 1
)
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The logic for initializing current_tool_id when it's -1 is unnecessarily complex. Consider simplifying:

if self.current_tool_id == -1:
    self.current_tool_id = 0
else:
    self.current_tool_id += 1

This achieves the same result as max(self.current_tool_id, 0) if self.current_tool_id == -1 else self.current_tool_id + 1 but is clearer and more readable.

Suggested change
self.current_tool_id = (
max(self.current_tool_id, 0) if self.current_tool_id == -1 else self.current_tool_id + 1
)
if self.current_tool_id == -1:
self.current_tool_id = 0
else:
self.current_tool_id += 1

Copilot uses AI. Check for mistakes.

if not self.model_tokenizer:
raise ValueError(
"The model tokenizer must be passed to the ReasoningParser " "constructor during construction."
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The error message string is split across two lines using string concatenation. Consider using a single string or parentheses for better readability:

raise ValueError(
    "The model tokenizer must be passed to the ReasoningParser constructor during construction."
)
Suggested change
"The model tokenizer must be passed to the ReasoningParser " "constructor during construction."
"The model tokenizer must be passed to the ReasoningParser constructor during construction."

Copilot uses AI. Check for mistakes.

if self.think_end_token_id is None:
raise RuntimeError(
"DeepSeek reasoning parser could not locate think end " "tokens in the tokenizer!"
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The error message string is split across two lines using string concatenation. Consider using a single string or parentheses for better readability:

raise RuntimeError(
    "DeepSeek reasoning parser could not locate think end tokens in the tokenizer!"
)
Suggested change
"DeepSeek reasoning parser could not locate think end " "tokens in the tokenizer!"
"DeepSeek reasoning parser could not locate think end tokens in the tokenizer!"

Copilot uses AI. Check for mistakes.
Comment on lines +49 to +55
"""
DeepSeek 系列模型的工具调用解析器(支持 V3.1、V3-0324、R1 三种模型)

支持的格式:
- V3.1: <|tool▁call▁begin|>function_name<|tool▁sep|>{"arg": "value"}<|tool▁call▁end|>
- V3-0324/R1: <|tool▁call▁begin|>function<|tool▁sep|>function_name\n```json\n{"arg": "value"}\n```<|tool▁call▁end|>
"""
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding an English translation to the docstring for consistency with the rest of the codebase. Many projects follow bilingual documentation standards. For example:

"""
DeepSeek series model tool call parser (supports V3.1, V3-0324, R1 models)
DeepSeek 系列模型的工具调用解析器(支持 V3.1、V3-0324、R1 三种模型)

Supported formats:
支持的格式:
- V3.1: <|tool▁call▁begin|>function_name<|tool▁sep|>{"arg": "value"}<|tool▁call▁end|>
- V3-0324/R1: <|tool▁call▁begin|>function<|tool▁sep|>function_name\n```json\n{"arg": "value"}\n```<|tool▁call▁end|>
"""

Copilot uses AI. Check for mistakes.
# 尝试使用 partial_json_parser
try:
args_dict = partial_json_parser.loads(function_arguments, flags=Allow.ALL)
except:
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using bare except: clauses. Catch specific exception types to ensure proper error handling and avoid masking unexpected errors. Consider catching Exception or specific parser exceptions:

except Exception:
    args_dict = {}

This applies to all three bare except: clauses in the file (lines 164, 294, 322).

Suggested change
except:
except Exception:

Copilot uses AI. Check for mistakes.
try:
args_dict = partial_json_parser.loads(args_text, flags=Allow.ALL)
args_str = json.dumps(args_dict, ensure_ascii=False)
except:
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using bare except: clauses. Catch specific exception types to ensure proper error handling and avoid masking unexpected errors. Consider catching Exception or specific parser exceptions:

except Exception:
    args_str = args_text
Suggested change
except:
except Exception:

Copilot uses AI. Check for mistakes.
# 使用 partial_json_parser 解析部分 JSON
args_dict = partial_json_parser.loads(args_text, flags=Allow.ALL)
args_str = json.dumps(args_dict, ensure_ascii=False)
except:
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using bare except: clauses. Catch specific exception types to ensure proper error handling and avoid masking unexpected errors. Consider catching Exception or specific parser exceptions:

except Exception:
    # 如果解析失败,直接使用原始文本
    args_str = args_text
Suggested change
except:
except Exception:

Copilot uses AI. Check for mistakes.
import json
import unittest

from fastdeploy.entrypoints.openai.protocol import ChatCompletionRequest, DeltaMessage
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'DeltaMessage' is not used.

Suggested change
from fastdeploy.entrypoints.openai.protocol import ChatCompletionRequest, DeltaMessage
from fastdeploy.entrypoints.openai.protocol import ChatCompletionRequest

Copilot uses AI. Check for mistakes.
@luotao1 luotao1 changed the title Hackathon 9th Sprint No.60】【RFC】为 FastDeploy 新增支持 DeepSeek 模型的 Reason ing Parser & Tool Parser 【Hackathon 9th Sprint No.60】【RFC】为 FastDeploy 新增支持 DeepSeek 模型的 Reason ing Parser & Tool Parser Dec 8, 2025
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 78.51240% with 52 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (develop@e43a5fc). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...points/openai/tool_parsers/deepseek_tool_parser.py 79.26% 20 Missing and 14 partials ⚠️
fastdeploy/reasoning/deepseek_reasoning_parser.py 76.92% 10 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #5412   +/-   ##
==========================================
  Coverage           ?   59.21%           
==========================================
  Files              ?      329           
  Lines              ?    40900           
  Branches           ?     6224           
==========================================
  Hits               ?    24219           
  Misses             ?    14801           
  Partials           ?     1880           
Flag Coverage Δ
GPU 59.21% <78.51%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants