feat: add ShengSuanYun as a provider for LLM and text embedding. #2175
Conversation
dosubot
bot
added
size:XXL
Summary of ChangesHello @coohu, 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 integrates ShengSuanYun as a new model provider, offering a broad selection of Large Language Models and text embedding capabilities. The integration includes detailed model configurations, custom API handling logic, and an automated publishing workflow, enhancing the platform's AI resource options and developer experience. Highlights
Ignored Files
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces ShengSuanYun as a new provider for LLM and text embedding models, adding a significant number of new models. The implementation, however, contains several critical issues in the Python code concerning credential handling, URL construction, and token calculation logic, which could lead to runtime errors. Additionally, there are minor inconsistencies in model labels within the YAML configuration files. I have provided detailed feedback and code suggestions to address these problems and improve the robustness and correctness of the new provider integration.
| credentials["endpoint_url"] = "https://router.shengsuanyun.com/api/v1" | ||
| if credentials.get("base_url"): | ||
| api_base = credentials["base_url"].rstrip("/") | ||
| credentials_kwargs["base_url"] = api_base + "/v1" |
There was a problem hiding this comment.
The variable credentials_kwargs is not defined in this scope, which will cause a NameError at runtime if a base_url is provided in the credentials. Additionally, the logic for constructing the URL is not robust, as it can lead to a malformed URL with a double /v1 suffix (e.g., .../v1/v1) if the provided base_url already includes it. The suggested change ensures the endpoint_url is constructed correctly, handling both cases and preventing runtime errors.
base_url = credentials.get("base_url")
if base_url:
api_base = base_url.rstrip("/")
if not api_base.endswith("/v1"):
credentials["endpoint_url"] = f"{api_base}/v1"
else:
credentials["endpoint_url"] = api_base
else:
credentials["endpoint_url"] = "https://router.shengsuanyun.com/api/v1"| assert prompt_messages[0].content is not None | ||
| prompt_tokens = self._num_tokens_from_string(model, prompt_messages[0].content) | ||
| assert assistant_message.content is not None | ||
| completion_tokens = self._num_tokens_from_string(model, assistant_message.content) |
There was a problem hiding this comment.
The fallback logic for token calculation is incorrect and will likely cause runtime errors.
prompt_tokensis calculated based only on the content of the first message (prompt_messages[0]). It should account for all messages in the prompt to be accurate.completion_tokensis calculated usingassistant_message.content, which is a list ofPromptMessageContentobjects, not a string. This will raise aTypeError. You should use theresponse_contentstring variable instead.
prompt_text = ""
for msg in prompt_messages:
if isinstance(msg.content, str):
prompt_text += msg.content
elif isinstance(msg.content, list):
for content_part in msg.content:
if isinstance(content_part, TextPromptMessageContent):
prompt_text += content_part.data
prompt_tokens = self._num_tokens_from_string(model, prompt_text)
completion_tokens = self._num_tokens_from_string(model, response_content)| credentials["extra_headers"] = { | ||
| "HTTP-Referer": "https://dify.ai/", | ||
| "X-Title": "Dify" | ||
| } |
There was a problem hiding this comment.
The extra_headers are added to the credentials dictionary, but this dictionary is not used by the openai.OpenAI client, which is initialized with credentials_kwargs. As a result, the headers will not be sent with the request. These headers should be added to credentials_kwargs using the default_headers key to ensure they are included in the API call.
| credentials["extra_headers"] = { | |
| "HTTP-Referer": "https://dify.ai/", | |
| "X-Title": "Dify" | |
| } | |
| credentials_kwargs["default_headers"] = { | |
| "HTTP-Referer": "https://dify.ai/", | |
| "X-Title": "Dify", | |
| } |
| @@ -0,0 +1,24 @@ | |||
| model: anthropic/claude-3.7-sonnet:thinking | |||
| label: | |||
| en_US: Claude-3.7-sonnet（Thinking） | |||
There was a problem hiding this comment.
The label uses full-width parentheses （）. For consistency with other labels, it's better to use standard half-width parentheses (). This ensures a uniform appearance across the UI.
en_US: Claude-3.7-sonnet (Thinking)| @@ -0,0 +1,24 @@ | |||
| model: anthropic/claude-sonnet-4:thinking | |||
| label: | |||
| en_US: Claude Sonnet 4 ( Thinking ) | |||
There was a problem hiding this comment.
The label contains unnecessary spaces around the word inside the parentheses. For better formatting and consistency, these spaces should be removed.
en_US: Claude Sonnet 4 (Thinking)| @@ -0,0 +1,24 @@ | |||
| model: deepseek/deepseek-v3.1-think | |||
| label: | |||
| en_US: ' DeepSeek V3.1 Think' | |||
There was a problem hiding this comment.
The label has a leading space, which can cause inconsistent display and potential issues with string matching. It should be removed.
en_US: DeepSeek V3.1 Think| @@ -0,0 +1,23 @@ | |||
| model: minimax/minimax-m2 | |||
| label: | |||
| en_US: 'MiniMax M2 ' | |||
There was a problem hiding this comment.
The label has a trailing space. This should be removed for consistency and to prevent potential string comparison issues.
en_US: MiniMax M2| @@ -0,0 +1,24 @@ | |||
| model: baidu/ernie-4.5-turbo-128k | |||
| label: | |||
| en_US: ' ERNIE-4.5-Turbo-128K' | |||
There was a problem hiding this comment.
The label has a leading space, which can cause inconsistent display and potential issues with string matching. It should be removed.
en_US: ERNIE-4.5-Turbo-128K| @@ -0,0 +1,23 @@ | |||
| model: bigmodel/glm-z1-airx | |||
| label: | |||
| en_US: GLM-Z1-AirX | |||
There was a problem hiding this comment.
The label contains leading zero-width spaces (U+200B), which are invisible but can cause issues with string matching and rendering. These should be removed.
en_US: GLM-Z1-AirX1 participant
Related Issues or Context
This PR contains Changes to Non-Plugin
This PR contains Changes to Non-LLM Models Plugin
This PR contains Changes to LLM Models Plugin
Version Control (Any Changes to the Plugin Will Require Bumping the Version)
VersionField, Not in Meta Section)Dify Plugin SDK Version
dify_plugin>=0.3.0,<0.6.0is in requirements.txt (SDK docs)Environment Verification (If Any Code Changes)
Local Deployment Environment
SaaS Environment