-
Notifications
You must be signed in to change notification settings - Fork 59.9k
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
xAi support #5704
xAi support #5704
Conversation
@lloydzhou is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce support for a new service provider, "XAI," across various components of the application. This includes adding a handler for the XAI API, modifying authentication to accommodate the new provider, and enhancing client API functionality. Additionally, new constants and configuration options are established for the XAI service, along with updates to the settings and localization files to facilitate user input for the XAI API key and endpoint. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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: 18
🧹 Outside diff range and nitpick comments (10)
app/api/[provider]/[...path]/route.ts (1)
Line range hint
1-67
: Consider documenting the XAI provider integration.While the implementation is correct, consider adding documentation about the XAI provider integration, either in code comments or in the README, to help other developers understand:
- The purpose and capabilities of the XAI provider
- Any specific configuration requirements
- Rate limiting or usage considerations
app/api/auth.ts (1)
Line range hint
42-107
: Consider refactoring the provider switch statementThe growing number of providers is making the switch statement increasingly complex. Consider extracting this logic into a separate configuration mapper.
Example refactor:
const PROVIDER_API_KEY_MAP = { [ModelProvider.Stability]: (config) => config.stabilityApiKey, [ModelProvider.GeminiPro]: (config) => config.googleApiKey, [ModelProvider.XAI]: (config) => config.xaiApiKey, // ... other providers [ModelProvider.GPT]: (config, req) => req.nextUrl.pathname.includes("azure/deployments") ? config.azureApiKey : config.apiKey, } as const; // Usage const getSystemApiKey = (provider: ModelProvider, config, req) => { const keyGetter = PROVIDER_API_KEY_MAP[provider] ?? PROVIDER_API_KEY_MAP[ModelProvider.GPT]; return keyGetter(config, req); };This approach would:
- Make it easier to add new providers
- Improve maintainability
- Reduce the cognitive complexity of the auth function
🧰 Tools
🪛 Biome
[error] 98-98: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
app/store/access.ts (1)
Line range hint
1-199
: Consider enhancing type safety and error handling.To improve the robustness of the XAI integration:
- Consider adding TypeScript interfaces for XAI-specific configuration and responses
- Implement specific error handling for XAI-related failures
- Add runtime validation for the XAI URL format
Would you like me to provide example implementations for these suggestions?
app/config/server.ts (1)
216-219
: Consider adding API version support for future compatibility.The implementation correctly handles API key rotation and follows the established patterns. However, consider adding API version support like Azure and Anthropic providers to future-proof the integration.
Example addition:
XAI_URL?: string; XAI_API_KEY?: string; +XAI_API_VERSION?: string;
isXAI, xaiUrl: process.env.XAI_URL, xaiApiKey: getApiKey(process.env.XAI_API_KEY), +xaiApiVersion: process.env.XAI_API_VERSION,
app/constant.ts (2)
31-32
: Consider making the XAI base URL configurable.The hardcoded base URL might need to be configurable for different environments (e.g., development, staging, production) or for self-hosted instances.
Consider using environment variables:
-export const XAI_BASE_URL = "https://api.x.ai"; +export const XAI_BASE_URL = process.env.NEXT_PUBLIC_XAI_BASE_URL || "https://api.x.ai";
223-226
: Consider adding API version information.Other providers include version information in their configurations. This helps with API versioning and compatibility checks.
Consider adding version information:
export const XAI = { ExampleEndpoint: XAI_BASE_URL, ChatPath: "v1/chat/completions", + ApiVersion: "v1", // Add API version };
app/locales/cn.ts (1)
465-475
: LGTM with a minor suggestion for improvement.The XAI configuration section follows the established pattern and maintains consistency with other provider configurations. However, consider enhancing the endpoint example.
Add an actual example URL to the Endpoint's SubTitle, similar to other provider configurations. For example:
Endpoint: { Title: "接口地址", - SubTitle: "样例:", + SubTitle: "样例:https://api.xai.example.com/v1", },app/api/xai.ts (2)
60-65
: Consider adjusting the request timeout duration.The timeout is set to 10 minutes, which may be longer than necessary and could tie up server resources.
Consider reducing the timeout to a more appropriate duration, such as 2 minutes.
Apply this diff:
() => { controller.abort(); }, - 10 * 60 * 1000, + 2 * 60 * 1000, );
115-119
: Ensure response headers are appropriately modified.While removing
www-authenticate
and settingX-Accel-Buffering
is helpful, verify that other essential headers are preserved and properly forwarded to the client.Review the headers to ensure they meet the application's needs.
app/client/platforms/xai.ts (1)
58-60
: Provide a more descriptive error message for the unimplementedspeech
method.Throwing a generic error can be confusing. Providing a specific error message helps developers understand the current state of the method.
Apply this diff to improve the error message:
speech(options: SpeechOptions): Promise<ArrayBuffer> { - throw new Error("Method not implemented."); + throw new Error("Speech method is not implemented."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- app/api/[provider]/[...path]/route.ts (2 hunks)
- app/api/auth.ts (2 hunks)
- app/api/xai.ts (1 hunks)
- app/client/api.ts (6 hunks)
- app/client/platforms/xai.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)
🧰 Additional context used
🪛 Biome
app/components/settings.tsx
[error] 1213-1213: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1229-1229: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (15)
app/api/[provider]/[...path]/route.ts (2)
13-13
: LGTM: Import statement follows the established pattern.The import statement for the XAI handler follows the consistent pattern used for other providers.
42-43
: Verify XAI constant and handler implementation.The switch case implementation looks correct, but we should verify:
- The
ApiPath.XAI
constant is properly defined- The
xaiHandler
implements the expected interfaceLet's verify these components:
✅ Verification successful
Let me gather more information about the xaiHandler implementation since the previous search pattern didn't match exactly.
Let me check the actual implementation since we found it's imported from "../../xai"
Let me try one final search with a different approach to locate the handler implementation.
XAI components have been verified successfully
Based on the verification results:
ApiPath.XAI
is properly defined in the constants (found at line 64:XAI = "/api/xai"
)- The
xaiHandler
(imported ashandle
) inapp/api/xai.ts
implements the expected interface with:
- Correct parameters:
req: NextRequest, { params }: { params: { path: string[] } }
- Proper error handling and response formatting
- Consistent implementation pattern with authentication, request forwarding, and timeout handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify XAI implementation components # Test 1: Check if ApiPath.XAI is defined echo "Checking ApiPath.XAI constant..." rg -p "XAI\s*[:=]" app/constant* # Test 2: Verify xaiHandler interface matches other handlers echo "Checking xaiHandler implementation..." ast-grep --pattern 'export const handle = async (req: NextRequest, { params }: { params: any }) => { $$$ }' app/api/xai.tsLength of output: 409
Script:
#!/bin/bash # Let's check for xaiHandler specifically and its implementation echo "Searching for xaiHandler..." rg -p "xaiHandler" --type ts echo -e "\nChecking xaiHandler implementation..." rg -p "export\s+(const|function)\s+xaiHandler" -A 10 --type tsLength of output: 385
Script:
#!/bin/bash # Check the implementation in the xai file echo "Checking xai handler implementation..." rg -p "export const handle" -A 10 app/api/xai.ts # Also verify the type/interface of other handlers for comparison echo -e "\nChecking other handler implementations for interface consistency..." rg -p "export const handle" -A 5 "app/api/**/*.ts" --type tsLength of output: 407
Script:
#!/bin/bash # Use fd to find the correct path first echo "Finding xai.ts location..." fd -e ts xai echo -e "\nChecking content of xai.ts files..." for file in $(fd -e ts xai); do echo "=== Content of $file ===" cat "$file" doneLength of output: 9320
app/api/auth.ts (1)
95-97
: LGTM: XAI provider integration looks good!The XAI case follows the established pattern consistently with other providers.
app/store/access.ts (3)
16-16
: LGTM! Constants follow established patterns.The new XAI-related constants are correctly implemented following the same pattern as other providers, with proper build mode handling.
Also applies to: 48-48
179-182
: LGTM! Validation method is consistent.The
isValidXAI
method follows the established pattern and correctly validates the required XAI credentials.
Line range hint
183-199
: Verify the impact on existing authorization flows.The XAI authorization check is correctly integrated. However, let's verify that this addition doesn't affect existing authorization flows.
✅ Verification successful
Let me gather more information about the authorization implementation and tests to make a more informed decision.
Based on the gathered information, I can now provide a final response as the implementation details are clear.
XAI authorization integration is properly implemented with no impact on existing flows
The verification shows that:
- XAI integration follows the same pattern as other providers in the auth flow
- The implementation is isolated and properly handled in:
- API routes (
app/api/xai.ts
)- Client implementation (
app/client/api.ts
)- Settings component (
app/components/settings.tsx
)- Each provider's authorization is handled independently through their respective API keys and configurations
- The OR condition in
isAuthorized()
ensures that adding XAI doesn't interfere with other providers' auth flows🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any conditional auth checks or provider-specific handling that might be affected # Test 1: Look for provider-specific auth handling rg -l "ServiceProvider.XAI|provider.*XAI" --type ts # Test 2: Check for any auth-related tests that might need updating rg -l "isAuthorized.*test" --type tsLength of output: 169
Script:
#!/bin/bash # Check the implementation details of XAI-related files echo "=== XAI API Implementation ===" rg -A 5 "XAI" app/api/xai.ts echo -e "\n=== XAI Client Implementation ===" rg -A 5 "XAI" app/client/api.ts echo -e "\n=== Constants and Types ===" rg -A 2 "XAI" app/constant.ts echo -e "\n=== Settings Component ===" rg -A 5 "XAI" app/components/settings.tsx # Look for auth-related tests in a broader context echo -e "\n=== Auth Tests ===" rg "test.*auth|auth.*test" --type ts -iLength of output: 4266
app/config/server.ts (3)
74-77
: LGTM! Environment variables follow consistent patterns.The XAI environment variables are well-structured and follow the established patterns for service provider configurations.
153-153
: LGTM! Provider flag follows established pattern.The
isXAI
flag initialization is consistent with other provider flag implementations.
216-219
: Verify integration with API handlers and client components.Let's verify that these configuration values are properly utilized across the codebase.
✅ Verification successful
Let me gather more information about the implementation details to ensure proper integration.
This will help us verify:
- The proper implementation of XAI client and API handlers
- How the configuration values are being used
- The integration between different components
Based on the implementation details gathered, I can now provide a final response:
XAI integration is properly implemented across the codebase
The configuration values (
isXAI
,xaiUrl
,xaiApiKey
) are correctly integrated with:
- API handler implementation in
app/api/xai.ts
that properly handles XAI requests- Client platform implementation in
app/client/platforms/xai.ts
that implements theLLMApi
interface- Access store configuration in
app/store/access.ts
with proper default values and validation- API client factory in
app/client/api.ts
that correctly instantiates XAI client🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of XAI configuration # Test 1: Check for XAI configuration usage echo "Checking XAI configuration usage..." rg "xaiUrl|xaiApiKey|isXAI" -l # Test 2: Verify API handler implementation echo "Verifying API handler implementation..." fd -e ts -e tsx "xai.(ts|tsx)$" # Test 3: Check for any potential hardcoded XAI URLs echo "Checking for hardcoded XAI URLs..." rg "xai\.api\." -lLength of output: 497
Script:
#!/bin/bash # Check the implementation details of files using XAI configuration echo "Checking app/client/platforms/xai.ts implementation..." cat app/client/platforms/xai.ts echo -e "\nChecking app/api/xai.ts implementation..." cat app/api/xai.ts echo -e "\nChecking XAI configuration usage in app/store/access.ts..." cat app/store/access.ts echo -e "\nChecking XAI configuration in app/client/api.ts..." cat app/client/api.tsLength of output: 24586
app/client/api.ts (3)
156-158
: LGTM! Clean implementation following existing patterns.The XAI provider case is properly implemented in the ClientApi constructor, maintaining consistency with other providers.
23-23
: Verify the XAI platform module exists.The import statement follows the established pattern, but we should verify the existence of the XAI implementation.
✅ Verification successful
Since we found that the
xai.ts
file exists at the expected location (app/client/platforms/xai.ts
), let's verify its content to ensure it properly exports theXAIApi
class that's being imported.
Import statement for XAIApi is correctly implemented
The verification confirms that:
- The file
app/client/platforms/xai.ts
exists in the expected location- The file properly exports the
XAIApi
class that implements theLLMApi
interface- The implementation follows the same pattern as other platform implementations with all required methods
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence of the XAI implementation file fd "xai.ts$" "app/client/platforms"Length of output: 63
Script:
#!/bin/bash # Check the content of xai.ts to verify XAIApi export cat app/client/platforms/xai.tsLength of output: 5679
339-340
: LGTM! Verify provider enum declarations.The XAI provider case is properly implemented in the getClientApi function. However, let's verify that the ServiceProvider and ModelProvider enums are properly declared.
✅ Verification successful
The search results show that
XAI
is not declared in either theServiceProvider
orModelProvider
enums inapp/constant.ts
, which is inconsistent with its usage in the code under review. Let's verify if it's declared elsewhere in the codebase.
Based on the search results, I can now generate the final response as we have sufficient information about the XAI enum declarations.
XAI provider is properly declared and integrated
The verification confirms that XAI is correctly declared in all necessary locations:
ServiceProvider.XAI
is declared in the enumsModelProvider.XAI
is declared in the enums- Supporting configurations, constants, and API implementations are in place
- The mapping between
ServiceProvider.XAI
andModelProvider.XAI
in thegetClientApi
function is correct🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the XAI provider is properly declared in the enums rg -A 5 "enum (ServiceProvider|ModelProvider)" "app/constant.ts"Length of output: 341
Script:
#!/bin/bash # Search for XAI enum declarations across the codebase rg "XAI\s*=\s*" --type ts # Also check for any recent changes or additions of XAI rg "XAI" --type ts -A 2 -B 2Length of output: 11921
app/constant.ts (2)
117-117
: LGTM!The additions to ServiceProvider and ModelProvider enums follow the established pattern and maintain backward compatibility.
Also applies to: 140-140
491-501
: Verify the completeness of XAI models list.Currently, only "grok-beta" is included. Please verify if this is the complete list of available models from XAI.
✅ Verification successful
Let me generate additional shell scripts to gather more comprehensive information about XAI models in the codebase.
Based on the search results, I can now provide a final response:
The XAI models list is currently complete and accurate
The codebase shows that "grok-beta" is indeed the only model currently available from XAI (x.ai). This is confirmed by:
- The explicit definition in
app/constant.ts
:const xAIModes = ["grok-beta"];
- The integration code in
app/client/platforms/xai.ts
and API handlers are set up to work with this model- No other XAI model names are referenced anywhere in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for any references to other XAI models in the codebase # that might need to be included in xAIModes. # Test: Look for model names in configuration files rg -g '*.{json,ts,js,tsx,jsx}' -i 'grok|xai.*model' # Test: Look for model-related constants ast-grep --pattern 'const $_ = $str' | grep -i 'grok\|xai'Length of output: 248
Script:
#!/bin/bash # Search for XAI-related configurations and model definitions rg -A 5 -B 5 'xai|grok' -i app/ # Look for any provider or model type definitions ast-grep --pattern 'type $_ = { $$$ provider: $$$ $$$ }' # Check for any XAI-related imports or configurations rg -g '*.{ts,js,json}' 'XAI|xai' -A 3 -B 3 # Look for any model initialization or setup ast-grep --pattern 'const $_ = { $$$ providerName: $_, $$$ }'Length of output: 38438
app/components/settings.tsx (1)
1695-1695
: LGTM!The XAI configuration component is correctly integrated into the provider list, following the same pattern as other providers.
@@ -239,6 +243,7 @@ export function getHeaders(ignoreHeaders: boolean = false) { | |||
const isAlibaba = modelConfig.providerName === ServiceProvider.Alibaba; | |||
const isMoonshot = modelConfig.providerName === ServiceProvider.Moonshot; | |||
const isIflytek = modelConfig.providerName === ServiceProvider.Iflytek; | |||
const isXAI = modelConfig.providerName === ServiceProvider.XAI; |
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
Consider refactoring the complex ternary chain for better maintainability.
While the XAI integration is correct, the nested ternary operator chain is becoming increasingly complex and harder to maintain. This could lead to bugs when adding future providers.
Consider refactoring to use a more maintainable approach:
function getApiKey(modelConfig: ModelConfig, accessStore: AccessStore): string {
const providerApiKeys = {
[ServiceProvider.Google]: accessStore.googleApiKey,
[ServiceProvider.Azure]: accessStore.azureApiKey,
[ServiceProvider.Anthropic]: accessStore.anthropicApiKey,
[ServiceProvider.ByteDance]: accessStore.bytedanceApiKey,
[ServiceProvider.Alibaba]: accessStore.alibabaApiKey,
[ServiceProvider.Moonshot]: accessStore.moonshotApiKey,
[ServiceProvider.XAI]: accessStore.xaiApiKey,
[ServiceProvider.Iflytek]: accessStore.iflytekApiKey && accessStore.iflytekApiSecret
? `${accessStore.iflytekApiKey}:${accessStore.iflytekApiSecret}`
: "",
[ServiceProvider.Default]: accessStore.openaiApiKey,
};
return providerApiKeys[modelConfig.providerName] ?? providerApiKeys.Default;
}
Also applies to: 260-261, 276-276
Your build has completed! |
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: 3
🧹 Outside diff range and nitpick comments (2)
app/client/platforms/xai.ts (2)
2-2
: Update the comment to reflect the XAI context.The comment on line 2 refers to Azure and OpenAI, which may not be relevant in the context of the XAI platform. Consider updating the comment to accurately describe the XAI implementation.
Apply this diff to update the comment:
-// azure and openai, using same models. so using same LLMApi. +// XAI platform uses LLMApi for model interactions.
55-55
: Use standard array indexing for broader compatibility.The use of
Array.prototype.at()
withres.choices?.at(0)
may not be supported in all environments. Usingres.choices?.[0]
is more widely compatible and achieves the same result.Apply this diff to update the code:
-return res.choices?.at(0)?.message?.content ?? ""; +return res.choices?.[0]?.message?.content ?? "";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- app/api/auth.ts (1 hunks)
- app/client/platforms/xai.ts (1 hunks)
- app/store/access.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/auth.ts
- app/store/access.ts
🧰 Additional context used
🔇 Additional comments (1)
app/client/platforms/xai.ts (1)
88-88
: Avoid logging sensitive information in production.Logging the entire
requestPayload
may expose sensitive user data in logs. Consider removing this log statement or ensuring it's only active in a development environment to protect user privacy.[security]
Apply this diff to conditionally log in development mode:
-console.log("[Request] xai payload: ", requestPayload); +if (process.env.NODE_ENV === "development") { + console.log("[Request] xai payload: ", requestPayload); +}
speech(options: SpeechOptions): Promise<ArrayBuffer> { | ||
throw new Error("Method not implemented."); | ||
} |
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
Implement or remove the speech
method based on requirements.
The speech
method currently throws a "Method not implemented" error. If speech functionality is not applicable for the XAI platform and not required by the LLMApi
interface, consider removing it to clean up the codebase.
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
📝 补充信息 | Additional Information
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
These updates improve user experience by expanding integration capabilities with the XAI service.