Add new model provider PPIO and bug fixes #6218
Conversation
|
@saikidev 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 integrates support for the new PPIO service provider throughout the application. Environment configuration files, documentation, localization resources, API routing, authentication, client-side API handling, and settings components were updated or added to enable PPIO functionality. Additionally, a dedicated API handler and a dedicated client API class (PPIOApi) were introduced, along with minor enhancements to chat summarization and stream processing. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as API Router (route.ts)
participant H as PPIO Handler (ppio.ts)
participant A as Auth Module (auth.ts)
C->>R: Request to /api/ppio endpoint
R-->>H: Route request based on ApiPath.PPIO
H->>A: Validate request using PPIO API key
A-->>H: Return authentication result
H->>H: Process request (fetch, error handling)
H-->>C: Return PPIO API response
sequenceDiagram
participant C as Client
participant CA as ClientApi (api.ts)
participant P as PPIOApi (platforms/ppio.ts)
C->>CA: Instantiate with ModelProvider.PPIO
CA->>P: Create new PPIOApi instance
C->>CA: Call chat(options)
CA->>P: Execute chat method
P-->>CA: Return chat response
Poem
β¨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
π§Ή Nitpick comments (11)
app/api/ppio.ts (4)
25-30: Ensure adequate error handling upon authentication failure.
WhenauthResult.erroris present, returning a 401 status is good. However, consider adding more context in the JSON payload (e.g., a short explanation or error code) to assist troubleshooting.
36-39: Add more specific error responses for debugging.
Catching and returning all errors withconsole.errorfollowed byprettyObject(e)may obscure root causes. Consider returning an error message or status code tailored to common failure cases (e.g., invalid URL, network error, JSON parse failure).
41-45: Clarify comment regarding 'alibaba' and 'base url'.
The comment says "alibaba use base url or just remove the path," but this code is for PPIO. Consider removing or updating this comment to avoid confusion for future maintainers.
118-128: Implement additional retry or fallback logic if needed.
The current approach terminates the request on timeout or fetch failure. Depending on how critical these requests are, consider implementing an optional retry or fallback mechanism to enhance reliability, especially under intermittent network conditions.app/client/platforms/ppio.ts (4)
1-3: Simplify or remove the inline comment about Azure/OpenAI.
This file references βazure and openaiβ in the comment but itβs specifically for PPIO. This might create confusion for maintainers. Consider removing or clarifying this comment.-// azure and openai, using same models. so using same LLMApi. +// PPIO uses the same LLMApi interface structure as other providers.
72-74: Implement or remove unimplementedspeechmethod.
Throwing an error for an unimplemented function can cause runtime issues if someone inadvertently calls it. If speech support is not planned, consider removing it or marking it with aTODOindicating future implementation.
109-110: Use professional language in comments.
The remark aboutmax_tokensmight be perceived as unprofessional. Swap out the phrase "this param is just shit" for a clearer explanationβor remove it entirelyβto maintain a more polished codebase.- // Please do not ask me why not send max_tokens, no reason, this param is just shit, I dont want to explain anymore. + // We skip sending max_tokens, as it causes unwanted limitation or side effects.
112-112: Update the console log label to reflect PPIO.
Currently, it prints β[Request] openai payload:β. Rename it to avoid confusion with different providers.- console.log("[Request] openai payload: ", requestPayload); + console.log("[Request] ppio payload: ", requestPayload);app/client/api.ts (1)
269-295: Consider reordering the API key assignment chain.The current implementation places the PPIO check after SiliconFlow but before Iflytek, which breaks the visual grouping of similar providers. Consider moving the PPIO check next to similar providers or at the end of the chain for better maintainability.
: isSiliconFlow ? accessStore.siliconflowApiKey - : isPPIO - ? accessStore.ppioApiKey : isIflytek ? accessStore.iflytekApiKey && accessStore.iflytekApiSecret ? accessStore.iflytekApiKey + ":" + accessStore.iflytekApiSecret : "" + : isPPIO + ? accessStore.ppioApiKey : accessStore.openaiApiKey;README.md (1)
363-370: Documentation for PPIO environment variables needs more details.While the basic structure is good, consider enhancing the documentation with:
- Example values (like other API configurations)
- Default values (if any)
- More detailed descriptions about PPIO service and its purpose
Apply this diff to improve the documentation:
### `PPIO_API_KEY` (optional) -PPIO API Key. +Your PPIO API key for authentication. +> Example: `ppio-xxxxxxxxxxxx` ### `PPIO_URL` (optional) -PPIO API URL. +Custom PPIO API endpoint URL. +> Default: `https://api.ppio.dev` +> Example: `https://your-ppio-proxy.com`app/store/chat.ts (1)
717-722: Consider adding constants and documentation for think tags.To improve maintainability and clarity:
- Define constants for the tag names.
- Add documentation explaining why think tags are removed from topics.
Apply this diff to implement the suggestions:
+// Constants for reasoning model tags +const THINK_START_TAG = "<think>"; +const THINK_END_TAG = "</think>"; + // deal with <think> and </think> tags -if (message.startsWith("<think>")) { +// Remove thinking process from topics to only keep the final conclusion +if (message.startsWith(THINK_START_TAG)) { message = message - .slice(message.indexOf("</think>") + 8) + .slice(message.indexOf(THINK_END_TAG) + THINK_END_TAG.length) .trim(); }
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (16)
.env.template(1 hunks)README.md(1 hunks)README_CN.md(1 hunks)app/api/[provider]/[...path]/route.ts(2 hunks)app/api/auth.ts(1 hunks)app/api/ppio.ts(1 hunks)app/client/api.ts(7 hunks)app/client/platforms/ppio.ts(1 hunks)app/components/settings.tsx(3 hunks)app/config/server.ts(3 hunks)app/constant.ts(7 hunks)app/locales/cn.ts(1 hunks)app/locales/en.ts(1 hunks)app/store/access.ts(5 hunks)app/store/chat.ts(1 hunks)app/utils/chat.ts(1 hunks)
π Additional comments (18)
app/api/ppio.ts (1)
10-11: Verifyauthusage for PPIO integration.
This looks correct for reusing the existing auth mechanism, but confirm that theModelProvider.PPIOlogic inauthcovers all PPIO-specific checks (e.g., presence ofppioApiKey) to prevent unauthorized requests.app/client/platforms/ppio.ts (2)
149-208: Gracefully handle JSON parse failures while streaming.
JSON parsing errors in the streaming callback can break the entire response flow. Consider adding a try/catch aroundJSON.parse(text)to handle malformed chunks or partial data, then emit a meaningful error or fallback logic.
246-278: ValidatedisableListModelslogic.
WhendisableListModelsistrue, returning only the default models is handy. Confirm that end users are aware that PPIO models wonβt be fetched in these scenarios, and log a warning if itβs unexpected.app/api/[provider]/[...path]/route.ts (1)
15-16: Well-structured provider registration.
AddingppioHandleris consistent with the existing provider pattern. Good job correctly placing it among the other handlers.Also applies to: 54-55
app/api/auth.ts (1)
107-109: LGTM!The implementation for the PPIO model provider follows the established pattern and is consistent with other provider implementations.
app/store/access.ts (1)
63-63: LGTM!The PPIO provider integration follows the established patterns:
- Proper URL configuration with environment-aware defaults
- Consistent state management in
DEFAULT_ACCESS_STATE- Validation method implementation
- Authorization check integration
Also applies to: 138-140, 229-231, 252-252
app/config/server.ts (1)
91-94: LGTM!The PPIO provider configuration is properly integrated:
- Type-safe environment variables in
ProcessEnv- Consistent flag implementation
- Proper configuration exposure in server config
Also applies to: 170-170, 254-257
app/client/api.ts (1)
27-27: LGTM!The PPIO provider integration follows the established patterns:
- Proper API class import
- Consistent provider initialization
- Client API configuration
- Service provider mapping
Also applies to: 172-174, 267-267, 289-291, 389-391
app/utils/chat.ts (1)
618-619: LGTM! Improved handling of newlines in thinking mode.The condition
chunk.content != "\n"ensures that single newlines after the<think>tag are properly handled, preventing unnecessary text accumulation in the output.app/locales/cn.ts (1)
510-520: LGTM! Added localization support for PPIO provider.The added localization strings for PPIO follow the established pattern used for other providers, maintaining consistency in the UI.
app/constant.ts (4)
39-39: LGTM! Added PPIO base URL constant.The base URL follows the standard format used for other providers.
269-273: LGTM! Added PPIO configuration object.The configuration object includes all necessary endpoints following the same structure as other providers.
654-674: LGTM! Added comprehensive list of PPIO models.The list includes a wide range of models from different providers (deepseek, qwen, meta-llama, etc.) with clear naming conventions.
832-842: LGTM! Added PPIO provider to DEFAULT_MODELS.The integration follows the established pattern, including proper provider configuration with a unique sort order.
app/locales/en.ts (1)
494-504: LGTM! Added English localization for PPIO provider.The English translations are consistent with the Chinese localization and follow the same pattern as other providers.
app/components/settings.tsx (1)
1363-1400: LGTM! PPIO configuration follows established patterns.The PPIO configuration component is well-structured and consistent with other provider implementations:
- Follows the same pattern for endpoint and API key configuration
- Uses appropriate input validation and state management
- Maintains UI consistency with other provider components
.env.template (1)
85-89: LGTM! Environment variables follow template conventions.The new PPIO environment variables are well-documented and follow the established pattern:
- Clear section headers
- Consistent naming convention
- Properly marked as optional
README_CN.md (1)
278-285: LGTM! Documentation follows established format.The PPIO configuration documentation is well-structured and consistent with other provider sections:
- Clear section headers
- Proper translation in Chinese
- Consistent formatting
app/api/ppio.ts
Outdated
| isModelNotavailableInServer( | ||
| serverConfig.customModels, | ||
| jsonBody?.model as string, | ||
| ServiceProvider.SiliconFlow as string, | ||
| ) |
There was a problem hiding this comment.
Double-check usage of ServiceProvider.SiliconFlow in PPIO code.
Using ServiceProvider.SiliconFlow here appears to be a mismatch. If this is intended to reference the PPIO service provider, update it to ServiceProvider.PPIO or an appropriate constant to avoid confusing or incorrect checks.
- ServiceProvider.SiliconFlow as string,
+ ServiceProvider.PPIO as string,π 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.
| isModelNotavailableInServer( | |
| serverConfig.customModels, | |
| jsonBody?.model as string, | |
| ServiceProvider.SiliconFlow as string, | |
| ) | |
| isModelNotavailableInServer( | |
| serverConfig.customModels, | |
| jsonBody?.model as string, | |
| ServiceProvider.PPIO as string, | |
| ) |
| // deal with <think> and </think> tags | ||
| if (message.startsWith("<think>")) { | ||
| message = message | ||
| .slice(message.indexOf("</think>") + 8) | ||
| .trim(); | ||
| } |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Add null check and error handling for think tags.
The implementation should handle edge cases where the closing tag is missing or malformed.
Apply this diff to improve error handling:
// deal with <think> and </think> tags
if (message.startsWith("<think>")) {
+ const endIndex = message.indexOf("</think>");
+ if (endIndex === -1) {
+ // Handle missing closing tag
+ message = message.slice("<think>".length).trim();
+ } else {
message = message
- .slice(message.indexOf("</think>") + 8)
+ .slice(endIndex + "</think>".length)
.trim();
+ }
}π 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.
| // deal with <think> and </think> tags | |
| if (message.startsWith("<think>")) { | |
| message = message | |
| .slice(message.indexOf("</think>") + 8) | |
| .trim(); | |
| } | |
| // deal with <think> and </think> tags | |
| if (message.startsWith("<think>")) { | |
| const endIndex = message.indexOf("</think>"); | |
| if (endIndex === -1) { | |
| // Handle missing closing tag | |
| message = message.slice("<think>".length).trim(); | |
| } else { | |
| message = message | |
| .slice(endIndex + "</think>".length) | |
| .trim(); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
π§Ή Nitpick comments (1)
app/api/ppio.ts (1)
60-65: Make request timeout configurable.The 10-minute timeout is hardcoded. Consider making it configurable through environment variables or server configuration.
Apply this diff to make the timeout configurable:
+const REQUEST_TIMEOUT_MS = Number(process.env.PPIO_REQUEST_TIMEOUT_MS) || 10 * 60 * 1000; + const timeoutId = setTimeout( () => { controller.abort(); }, - 10 * 60 * 1000, + REQUEST_TIMEOUT_MS, );
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
app/api/ppio.ts(1 hunks)
π Additional comments (2)
app/api/ppio.ts (2)
1-13: LGTM!The imports and configuration setup are well-organized and include all necessary dependencies.
81-110: LGTM!The model filtering logic is well-implemented with proper error handling and access control.
| try { | ||
| const response = await request(req); | ||
| return response; | ||
| } catch (e) { | ||
| console.error("[PPIO] ", e); | ||
| return NextResponse.json(prettyObject(e)); | ||
| } |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Enhance error handling.
The current error handling could expose sensitive information and doesn't provide appropriate HTTP status codes.
Apply this diff to improve error handling:
} catch (e) {
console.error("[PPIO] ", e);
- return NextResponse.json(prettyObject(e));
+ return NextResponse.json(
+ {
+ error: true,
+ message: "An error occurred while processing your request",
+ },
+ { 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.
| try { | |
| const response = await request(req); | |
| return response; | |
| } catch (e) { | |
| console.error("[PPIO] ", e); | |
| return NextResponse.json(prettyObject(e)); | |
| } | |
| try { | |
| const response = await request(req); | |
| return response; | |
| } catch (e) { | |
| console.error("[PPIO] ", e); | |
| return NextResponse.json( | |
| { | |
| error: true, | |
| message: "An error occurred while processing your request", | |
| }, | |
| { status: 500 } | |
| ); | |
| } |
| try { | ||
| const res = await fetch(fetchUrl, fetchOptions); | ||
|
|
||
| // to prevent browser prompt for credentials | ||
| const newHeaders = new Headers(res.headers); | ||
| newHeaders.delete("www-authenticate"); | ||
| // to disable nginx buffering | ||
| newHeaders.set("X-Accel-Buffering", "no"); | ||
|
|
||
| return new Response(res.body, { | ||
| status: res.status, | ||
| statusText: res.statusText, | ||
| headers: newHeaders, | ||
| }); | ||
| } finally { | ||
| clearTimeout(timeoutId); | ||
| } | ||
| } |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Improve fetch error handling.
The fetch call could fail silently without proper error handling for network failures or timeouts.
Apply this diff to improve error handling:
try {
const res = await fetch(fetchUrl, fetchOptions);
+ if (!res.ok) {
+ throw new Error(`HTTP error! status: ${res.status}`);
+ }
// to prevent browser prompt for credentials
const newHeaders = new Headers(res.headers);
newHeaders.delete("www-authenticate");
// to disable nginx buffering
newHeaders.set("X-Accel-Buffering", "no");
return new Response(res.body, {
status: res.status,
statusText: res.statusText,
headers: newHeaders,
});
+ } catch (error) {
+ if (error.name === 'AbortError') {
+ throw new Error('Request timed out');
+ }
+ throw error;
} finally {
clearTimeout(timeoutId);
}π 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.
| try { | |
| const res = await fetch(fetchUrl, fetchOptions); | |
| // to prevent browser prompt for credentials | |
| const newHeaders = new Headers(res.headers); | |
| newHeaders.delete("www-authenticate"); | |
| // to disable nginx buffering | |
| newHeaders.set("X-Accel-Buffering", "no"); | |
| return new Response(res.body, { | |
| status: res.status, | |
| statusText: res.statusText, | |
| headers: newHeaders, | |
| }); | |
| } finally { | |
| clearTimeout(timeoutId); | |
| } | |
| } | |
| try { | |
| const res = await fetch(fetchUrl, fetchOptions); | |
| if (!res.ok) { | |
| throw new Error(`HTTP error! status: ${res.status}`); | |
| } | |
| // to prevent browser prompt for credentials | |
| const newHeaders = new Headers(res.headers); | |
| newHeaders.delete("www-authenticate"); | |
| // to disable nginx buffering | |
| newHeaders.set("X-Accel-Buffering", "no"); | |
| return new Response(res.body, { | |
| status: res.status, | |
| statusText: res.statusText, | |
| headers: newHeaders, | |
| }); | |
| } catch (error) { | |
| if (error.name === 'AbortError') { | |
| throw new Error('Request timed out'); | |
| } | |
| throw error; | |
| } finally { | |
| clearTimeout(timeoutId); | |
| } | |
| } |
π» εζ΄η±»ε | Change Type
π εζ΄θ―΄ζ | Description of Change
π θ‘₯ε δΏ‘ζ― | Additional Information
Summary by CodeRabbit
New Features
Improvements