-
Notifications
You must be signed in to change notification settings - Fork 59.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
since #5984, add DeepSeek as a new ModelProvider (with deepseek-chat&deepseek-coder models), so that user can use openai and deepseek at same time with different api url & key #5989
base: main
Are you sure you want to change the base?
Conversation
…deepseek-chat&deepseek-corder models), so that user can use openai and deepseek at same time with different api url&key
@bestsanmao is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces comprehensive support for the DeepSeek AI service across multiple components of the application. The changes span several files, adding new routing logic, authentication handling, API client integration, and configuration settings to enable seamless interaction with the DeepSeek API. The implementation includes defining new constants, updating enums, creating a new API handler, and modifying existing client-side and server-side configurations to accommodate the new service provider. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
app/api/deepseek.ts (2)
19-23
: Add usage clarifications for preflight response.
This block correctly handles preflight (OPTIONS) requests by returning a 200 response, but it might help future maintainers to include a brief comment explaining that this is needed for CORS or streaming-based requests.
60-66
: Evaluate prolonged 10-minute abort timeout.
While a 10-minute timeout can handle long-running requests, consider whether your application needs an even longer or shorter threshold to optimize resource usage. Extremely long requests might consume server resources or lead to client confusion.app/client/platforms/deepseek.ts (2)
32-57
: Refine fallback logic for the DeepSeek endpoint.
Thepath
method checksaccessStore.moonshotUrl
first before defaulting toDEEPSEEK_BASE_URL
orApiPath.DeepSeek
. Ensure that this fallback mechanism is aligned with user expectations. If there's a separate config for DeepSeek, referencingmoonshotUrl
might be confusing.
63-65
: Implement or remove the unfinished "speech" method.
speech
currently throws an error. Either fully implement text-to-speech for DeepSeek or remove the method to reduce confusion and potential runtime errors.app/store/access.ts (1)
114-117
: Initialize DeepSeek credentials with caution.
deepseekUrl
anddeepseekApiKey
are now part ofDEFAULT_ACCESS_STATE
. This ensures consistent structure, but confirm if there's a fallback or user prompt if the key isn't set.app/config/server.ts (1)
74-75
: Add environment variable usage remindersThese new environment variables for DeepSeek (
DEEPSEEK_URL
andDEEPSEEK_API_KEY
) are consistent with other integrations. If you haven't already, remember to document them in your project’s environment variable reference or.env.example
file, so future contributors and users know to set them properly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/api/[provider]/[...path]/route.ts
(2 hunks)app/api/auth.ts
(1 hunks)app/api/deepseek.ts
(1 hunks)app/client/api.ts
(7 hunks)app/client/platforms/deepseek.ts
(1 hunks)app/config/server.ts
(4 hunks)app/constant.ts
(7 hunks)app/store/access.ts
(5 hunks)
🔇 Additional comments (27)
app/api/deepseek.ts (3)
25-30
: Confirm error payload structure for auth failures.
The current logic returns a JSON object with authResult.error
and authResult.msg
fields upon 401. Verify that the client-side code checks the same fields and can interpret them correctly.
81-110
: Validate the "not allowed model" logic.
Your check ensures that some models are disallowed if present in the custom models. Double-check that the correct service provider (ServiceProvider.Moonshot as string
) is referenced here, since the code is in a DeepSeek context. Ensure you have the correct logic to guard usage of the right models.
114-125
: Ensure "X-Accel-Buffering" works as intended in all environments.
The header X-Accel-Buffering: no
is typically recognized by Nginx for disabling response buffering. Confirm your reverse proxy or server environment supports this directive, or consider removing it to avoid confusion.
app/client/platforms/deepseek.ts (2)
74-93
: Confirm alignment between chosen model and provider.
These lines merge custom config from multiple sources into modelConfig
. Ensure that options.config.model
cannot conflict with the providerName
in other parts of the code, especially if placeholders or default fallback logic is used.
179-184
: Good usage of the non-stream case.
The non-stream flow properly handles response JSON, extracts the user message, and fires the finish callback. This looks clean and consistent with your streaming approach.
app/api/[provider]/[...path]/route.ts (2)
13-13
: Align your import naming with usage.
The import statement import { handle as deepseekHandler }
is consistent with other handler imports. This enhances code clarity. No issues here.
44-45
: DeepSeek routing looks good.
The new case for ApiPath.DeepSeek
integrates seamlessly into the existing switch. Ensure the appended route matches server configurations so it doesn't overlap with other providers.
[approve]
app/api/auth.ts (1)
95-97
: Ensure deepseekApiKey
is populated in server config.
When injecting the DeepSeek system API key, confirm that serverConfig.deepseekApiKey
is properly set in production. Otherwise, Authorization
might become an empty bearer token.
app/store/access.ts (2)
51-52
: Confirm usage of DEFAULT_DEEPSEEK_URL
.
By defaulting deepseekUrl
to ApiPath.DeepSeek
for the non-app build mode, you rely on the routing logic. Verify that users expect to route via /api/deepseek
or an external URL.
193-195
: New isValidDeepSeek
check integrated into isAuthorized
.
The method isValidDeepSeek
ensures deepseekApiKey
is non-empty when validating overall app access. Confirm that partial or other forms of auth do not inadvertently bypass or block DeepSeek usage.
Also applies to: 220-220
app/config/server.ts (3)
135-137
: Logic consistently filters GPT-4-like models
These lines extend the GPT-4 filtering logic to additional model name patterns. No functional issues are apparent. Ensure that filters for other newly introduced providers remain similarly well-documented for cross-team consistency.
163-163
: DeepSeek environment check
The isDeepSeek
boolean is consistent with the pattern used for other providers (isAnthropic
, isStability
, etc.). This helps keep the provider check uniform. Good addition!
228-231
: New DeepSeek config entries added correctly
Including isDeepSeek
, deepseekUrl
, and deepseekApiKey
in the returned server configuration aligns with existing patterns for other providers. Be sure to confirm that references to these properties, especially deepseekApiKey
, handle potential scenarios where process.env.DEEPSEEK_API_KEY
might be unset.
app/client/api.ts (7)
23-23
: Import statement for DeepSeekApi
Good addition. Make sure the DeepSeekApi
file path is correct and that it properly exports the class.
158-160
: DeepSeek provider implemented in ClientApi
This creates a new instance of DeepSeekApi
when ModelProvider.DeepSeek
is selected. It follows the pattern of other providers neatly. Consider testing for correct instantiation with different environment settings.
254-254
: DeepSeek providerName check
Consistent with how other providers are handled. This ensures that the code picks up the correct provider in the getConfig
function.
272-273
: Accessing DeepSeek API Key
It’s good to see you’re retrieving the DeepSeek API key from the accessStore
here. Double-check that accessStore.deepseekApiKey
can handle fallback logic if this key is missing or empty.
290-290
: isDeepSeek in the destructured config
Adding isDeepSeek
aligns with existing flags for other providers. Properly used for controlling whether to set specific DeepSeek headers.
313-319
: Destructured flags for additional clarity
Including isDeepSeek
in the same destructuring statement as other providers helps keep the logic well-organized. No further issues identified.
362-363
: New getClientApi branch for DeepSeek
Providing a dedicated case for the DeepSeek
service is consistent with how other providers are handled. This ensures the correct client instance is returned.
app/constant.ts (7)
31-32
: DeepSeek base URL defined
Having a dedicated DEEPSEEK_BASE_URL
constant matches the pattern for other providers, enhancing maintainability.
70-70
: ApiPath extended to support DeepSeek
Defining /api/deepseek
in ApiPath
is straightforward. Ensure the new endpoint is routed correctly.
125-125
: ServiceProvider extended
Adding DeepSeek
to ServiceProvider
works consistently. Double-check references in any UI or settings components that might rely on enumerating all providers.
150-150
: ModelProvider extended
Similar to the ServiceProvider
addition, including DeepSeek
here keeps naming consistent across the codebase.
233-237
: DeepSeek constant block
Exporting a constant for DeepSeek
with an ExampleEndpoint
and ChatPath
helps unify references to the service. This is consistent with other providers.
433-434
: DeepSeek model array
Declaring deepseekModels
now ensures easy extension if new models appear later. No issues.
582-592
: DEFAULT_MODELS expansion for DeepSeek
Appending the mapped DeepSeek models to DEFAULT_MODELS
ensures the system recognizes and displays them. Confirm that sorting logic (sequence) is correct and not conflicting with other providers’ order.
const chatPath = this.path(DeepSeek.ChatPath); | ||
const chatPayload = { | ||
method: "POST", | ||
body: JSON.stringify(requestPayload), | ||
signal: controller.signal, | ||
headers: getHeaders(), | ||
}; | ||
|
||
// make a fetch request | ||
const requestTimeoutId = setTimeout( | ||
() => controller.abort(), | ||
REQUEST_TIMEOUT_MS, | ||
); | ||
|
||
if (shouldStream) { | ||
const [tools, funcs] = usePluginStore | ||
.getState() | ||
.getAsTools( | ||
useChatStore.getState().currentSession().mask?.plugin || [], | ||
); | ||
return stream( | ||
chatPath, | ||
requestPayload, | ||
getHeaders(), | ||
tools as any, | ||
funcs, | ||
controller, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simplify streaming callback structure.
This block handles chunked SSE data with tool calls. Consider extracting the tool-handling logic into a dedicated helper function for improved readability and separation of concerns.
please resolve the conflicts |
…-Web into add_deepseek
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/constant.ts (1)
436-436
: Consider adding knowledge cutoff dates for DeepSeek models.While the model configuration is correct, consider adding knowledge cutoff dates for DeepSeek models in the KnowledgeCutOffDate record to ensure consistent behavior with other providers.
export const KnowledgeCutOffDate: Record<string, string> = { default: "2021-09", + "deepseek-chat": "2023-12", + "deepseek-coder": "2023-12", // ... existing entries };Also applies to: 594-604
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/api/common.ts
(1 hunks)app/config/server.ts
(3 hunks)app/constant.ts
(7 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/api/common.ts
🔇 Additional comments (6)
app/config/server.ts (3)
75-76
: LGTM! Environment variables follow established patterns.
The addition of DEEPSEEK environment variables maintains consistency with other provider configurations.
154-154
: LGTM! Provider flag follows established patterns.
The isDeepSeek flag implementation maintains consistency with other provider flags.
219-221
: LGTM! Configuration properties properly integrated.
The DeepSeek configuration properties are well-integrated and properly handle API key rotation through the getApiKey helper.
app/constant.ts (3)
31-31
: LGTM! API endpoint configuration is correct.
The DeepSeek API endpoint and path configurations follow the established patterns and point to the correct official endpoints.
Also applies to: 70-70
125-125
: LGTM! Provider enums properly extended.
The addition of DeepSeek to both ServiceProvider and ModelProvider enums maintains consistency with the existing implementation.
Also applies to: 150-150
233-236
: Verify the DeepSeek API endpoint structure.
While the configuration follows the established pattern, please verify that the ChatPath "chat/completions" matches DeepSeek's official API documentation.
✅ Verification successful
DeepSeek API endpoint structure is correctly implemented
The verification of the codebase confirms that the DeepSeek API endpoint structure is correctly implemented:
- The base URL is properly configured as
https://api.deepseek.com
- The chat endpoint path
chat/completions
is consistent with other LLM providers in the codebase - The implementation in
app/client/platforms/deepseek.ts
shows proper handling of the API endpoint construction and request routing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any existing DeepSeek API endpoint usage to verify consistency
rg -l 'chat/completions.*deepseek' || echo "No existing DeepSeek API endpoint references found"
Length of output: 147
Script:
#!/bin/bash
# Let's search for any DeepSeek-related configurations or implementations
rg -i "deepseek" -A 5 -B 5
Length of output: 20490
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
app/api/deepseek.ts (3)
44-45
: Improve URL path manipulation robustness.The current path manipulation using string replacement could be fragile. Consider using URL manipulation utilities.
- let path = `${req.nextUrl.pathname}`.replaceAll(ApiPath.DeepSeek, ""); + const path = new URL(req.url).pathname.replace(new RegExp(`^${ApiPath.DeepSeek}`), '');
60-65
: Extract timeout duration as a constant.The 10-minute timeout value should be defined as a named constant for better maintainability.
+ const REQUEST_TIMEOUT_MS = 10 * 60 * 1000; // 10 minutes const timeoutId = setTimeout( () => { controller.abort(); }, - 10 * 60 * 1000, + REQUEST_TIMEOUT_MS, );
111-128
: Add specific error handling for network issues.While the basic error handling is present via the outer try-catch, it would be beneficial to handle specific network-related errors here.
try { const res = await fetch(fetchUrl, fetchOptions); + if (!res.ok) { + const errorData = await res.text().catch(() => null); + throw new Error(`DeepSeek API responded with status ${res.status}: ${errorData || res.statusText}`); + } const newHeaders = new Headers(res.headers);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/api/deepseek.ts
(1 hunks)app/constant.ts
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/constant.ts
🔇 Additional comments (1)
app/api/deepseek.ts (1)
1-14
: LGTM! Imports and configuration setup look good.
The necessary dependencies are properly imported and the server configuration is retrieved at the module level.
// @ts-ignore | ||
duplex: "half", | ||
signal: controller.signal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove @ts-ignore and properly type the duplex option.
The @ts-ignore
comment suggests a typing issue that should be addressed properly.
Consider creating a proper type for the fetch options:
type ExtendedRequestInit = RequestInit & {
duplex: 'half' | 'full';
};
const fetchOptions: ExtendedRequestInit = {
// ... other options ...
duplex: 'half',
};
export async function handle( | ||
req: NextRequest, | ||
{ params }: { params: { path: string[] } }, | ||
) { | ||
console.log("[DeepSeek Route] params ", params); | ||
|
||
if (req.method === "OPTIONS") { | ||
return NextResponse.json({ body: "OK" }, { status: 200 }); | ||
} | ||
|
||
const authResult = auth(req, ModelProvider.DeepSeek); | ||
if (authResult.error) { | ||
return NextResponse.json(authResult, { | ||
status: 401, | ||
}); | ||
} | ||
|
||
try { | ||
const response = await request(req); | ||
return response; | ||
} catch (e) { | ||
console.error("[DeepSeek] ", e); | ||
return NextResponse.json(prettyObject(e)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error handling in the handle function.
While the basic flow is correct, the error handling could be improved:
Consider applying this improvement:
} catch (e) {
console.error("[DeepSeek] ", e);
- return NextResponse.json(prettyObject(e));
+ return NextResponse.json(
+ {
+ error: true,
+ message: e instanceof Error ? e.message : "Unknown error occurred",
+ details: prettyObject(e)
+ },
+ { status: 500 }
+ );
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export async function handle( | |
req: NextRequest, | |
{ params }: { params: { path: string[] } }, | |
) { | |
console.log("[DeepSeek Route] params ", params); | |
if (req.method === "OPTIONS") { | |
return NextResponse.json({ body: "OK" }, { status: 200 }); | |
} | |
const authResult = auth(req, ModelProvider.DeepSeek); | |
if (authResult.error) { | |
return NextResponse.json(authResult, { | |
status: 401, | |
}); | |
} | |
try { | |
const response = await request(req); | |
return response; | |
} catch (e) { | |
console.error("[DeepSeek] ", e); | |
return NextResponse.json(prettyObject(e)); | |
} | |
} | |
export async function handle( | |
req: NextRequest, | |
{ params }: { params: { path: string[] } }, | |
) { | |
console.log("[DeepSeek Route] params ", params); | |
if (req.method === "OPTIONS") { | |
return NextResponse.json({ body: "OK" }, { status: 200 }); | |
} | |
const authResult = auth(req, ModelProvider.DeepSeek); | |
if (authResult.error) { | |
return NextResponse.json(authResult, { | |
status: 401, | |
}); | |
} | |
try { | |
const response = await request(req); | |
return response; | |
} catch (e) { | |
console.error("[DeepSeek] ", e); | |
return NextResponse.json( | |
{ | |
error: true, | |
message: e instanceof Error ? e.message : "Unknown error occurred", | |
details: prettyObject(e) | |
}, | |
{ status: 500 } | |
); | |
} | |
} |
const fetchUrl = `${baseUrl}${path}`; | ||
const fetchOptions: RequestInit = { | ||
headers: { | ||
"Content-Type": "application/json", | ||
Authorization: req.headers.get("Authorization") ?? "", | ||
}, | ||
method: req.method, | ||
body: req.body, | ||
redirect: "manual", | ||
// @ts-ignore | ||
duplex: "half", | ||
signal: controller.signal, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Review of sensitive information handling reveals potential logging concerns
The code review reveals consistent handling of Authorization headers across all API files, which is good. However, there are potential security concerns with logging:
- All API files log request parameters (
params
) which might contain sensitive information - Base URLs and paths are logged, which could expose API endpoints
- Error logging might potentially expose sensitive details
Recommendations:
- Remove or redact sensitive information from parameter logging
- Consider using a proper logging framework with different log levels
- Implement sanitization of error messages before logging
🔗 Analysis chain
Verify secure handling of sensitive information.
Let's ensure that sensitive information is properly handled in the request headers and logging.
Let me check the authentication header handling in the other files to ensure consistent and secure practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential sensitive data exposure in logging statements
rg -n "console\." app/api/deepseek.ts
# Check if authentication headers are properly handled elsewhere in the codebase
rg -l "Authorization.*header" --type ts
Length of output: 430
Script:
#!/bin/bash
# Check how Authorization header is handled in other API files
for file in app/api/xai.ts app/api/iflytek.ts app/api/glm.ts app/api/moonshot.ts app/api/bytedance.ts app/api/alibaba.ts; do
echo "=== $file ==="
rg -A 2 "Authorization.*header" "$file"
done
# Check for any sensitive data logging in these files
for file in app/api/xai.ts app/api/iflytek.ts app/api/glm.ts app/api/moonshot.ts app/api/bytedance.ts app/api/alibaba.ts; do
echo "=== $file ==="
rg "console\." "$file"
done
Length of output: 4569
since #5984, add DeepSeek as a new ModelProvider (with deepseek-chat&deepseek-corder models), so that user can use openai and deepseek at same time with different api url&key
Summary by CodeRabbit
Release Notes
New Features
DeepSeekApi
class for chat interactions and API calls.Configuration Updates
Access Store Enhancements