-
Notifications
You must be signed in to change notification settings - Fork 59.3k
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
support one-click fetch available models in OpenAI Channel #4815
Conversation
WalkthroughThe recent updates introduce a feature to fetch available models from the OpenAI API within the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant SettingsComponent
participant OpenAIAPI
participant LocaleStore
participant AccessStore
User->>SettingsComponent: Click "Fetch Available Models"
SettingsComponent->>LocaleStore: Get confirmation message
LocaleStore-->>SettingsComponent: Return confirmation message
SettingsComponent->>User: Display confirmation dialog
User->>SettingsComponent: Confirm action
SettingsComponent->>AccessStore: Call fetchAvailableModels function
AccessStore->>OpenAIAPI: Fetch available models
OpenAIAPI-->>AccessStore: Return available models
AccessStore-->>SettingsComponent: Provide formatted model IDs
SettingsComponent->>User: Display fetched models
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
@code-october is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- app/components/settings.tsx (1 hunks)
- app/locales/cn.ts (1 hunks)
- app/locales/en.ts (1 hunks)
- app/store/access.ts (1 hunks)
Additional context used
Biome
app/store/access.ts
[error] 133-133: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 142-142: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 145-145: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 180-180: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
app/locales/cn.ts
[error] 290-290: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 349-349: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 349-349: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 506-506: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
app/locales/en.ts
[error] 224-224: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 269-269: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 270-270: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 413-413: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
app/components/settings.tsx
[error] 103-103: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 115-115: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 338-338: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 359-359: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 374-374: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 392-392: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 405-405: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 416-416: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 434-434: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 448-448: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 458-458: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 680-680: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 688-688: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 695-700: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
[error] 737-737: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 737-737: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 754-754: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 754-754: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 770-770: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 794-794: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
Additional comments not posted (4)
app/locales/cn.ts (1)
298-304
: Localization entries forAvailableModels
are correctly implemented.The new entries provide clear instructions and actions for the user, enhancing the user experience for Chinese-speaking users.
app/locales/en.ts (1)
301-307
: Localization entries forAvailableModels
are correctly implemented.The new entries provide clear instructions and actions for the user, enhancing the user experience for English-speaking users.
app/components/settings.tsx (2)
1006-1019
: Ensure proper error handling in the new model fetching functionality.Verification successful
The
fetchAvailableModels
function inapp/store/access.ts
does include error handling using a.catch
block. This block logs an error message and returns a default value (DEFAULT_ACCESS_STATE.customModels
) if the fetch operation fails. Therefore, the new model fetching functionality inapp/components/settings.tsx
does have error handling through thefetchAvailableModels
function.
- Location of
fetchAvailableModels
function definition:
app/store/access.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper error handling in the new model fetching functionality. # Test: Search for error handling patterns around the fetchAvailableModels call. rg --type typescript $'try {.*fetchAvailableModels.*} catch'Length of output: 185
Script:
#!/bin/bash # Description: Verify proper error handling in the new model fetching functionality. # Test: Search for error handling patterns around the fetchAvailableModels call. rg --type typescript 'try {.*fetchAvailableModels.*} catch'Length of output: 185
Script:
#!/bin/bash # Description: Verify proper error handling in the new model fetching functionality. # Test: Search for error handling patterns around the fetchAvailableModels call. rg --type typescript 'try \{.*fetchAvailableModels.*\} catch'Length of output: 98
Script:
#!/bin/bash # Description: Verify proper error handling in the new model fetching functionality. # Test: Search for error handling patterns around the fetchAvailableModels call. rg 'try \{.*fetchAvailableModels.*\} catch'Length of output: 45
Script:
#!/bin/bash # Description: Verify the presence and usage of the fetchAvailableModels function. # Search for the definition of the fetchAvailableModels function. rg 'function fetchAvailableModels' # Search for the usage of the fetchAvailableModels function. rg 'fetchAvailableModels'Length of output: 328
Script:
#!/bin/bash # Description: Extract the fetchAvailableModels function definition for review. # Extract the fetchAvailableModels function definition from app/store/access.ts. rg --context 10 'fetchAvailableModels' app/store/access.tsLength of output: 765
Script:
#!/bin/bash # Description: Extract the complete fetchAvailableModels function definition for review. # Extract the complete fetchAvailableModels function definition from app/store/access.ts. rg --context 50 'fetchAvailableModels' app/store/access.tsLength of output: 3312
1015-1015
: Ensure that theconfig.update
method properly handles the new model IDs.Verification successful
The
createPersistStore
function inapp/utils/store.ts
defines anupdate
method that clones the current state, applies the updater function, and then sets the updated state with a newlastUpdateTime
. This confirms that theconfig.update
method properly handles updates, including new model IDs.
app/utils/store.ts
:export function createPersistStore<T extends object, M>( state: T, methods: ( set: SetStoreState<T & MakeUpdater<T>>, get: () => T & MakeUpdater<T>, ) => M, persistOptions: SecondParam<typeof persist<T & M & MakeUpdater<T>>>, ) { return create( persist( combine( { ...state, lastUpdateTime: 0, }, (set, get) => { return { ...methods(set, get as any), markUpdate() { set({ lastUpdateTime: Date.now() } as Partial< T & M & MakeUpdater<T> >); }, update(updater) { const state = deepClone(get()); updater(state); set({ ...state, lastUpdateTime: Date.now(), }); }, } as M & MakeUpdater<T>; }, ), persistOptions as any, ), ); }This implementation ensures that the
update
method can handle new model IDs correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the config.update method properly handles the new model IDs. # Test: Search for the implementation of config.update to ensure it can handle model IDs. rg --type typescript $'config.update'Length of output: 71
Script:
#!/bin/bash # Description: Verify that the config.update method properly handles the new model IDs. # Test: Search for the implementation of config.update to ensure it can handle model IDs. rg 'config.update' --glob '*.ts' --glob '*.tsx'Length of output: 725
Script:
#!/bin/bash # Description: Locate the definition of the update method within the config object. # Test: Search for the definition of the update method in TypeScript files. rg 'update' --glob '*.ts' --glob '*.tsx' | grep -i 'config'Length of output: 2937
Script:
#!/bin/bash # Description: Locate the definition of the config object to find the update method implementation. # Test: Search for the definition of the config object in TypeScript files. rg 'config' --glob '*.ts' --glob '*.tsx'Length of output: 21963
Script:
#!/bin/bash # Description: Locate the definition of the useAppConfig function to find the config object implementation. # Test: Search for the definition of the useAppConfig function in TypeScript files. rg 'useAppConfig' --glob '*.ts' --glob '*.tsx'Length of output: 3223
Script:
#!/bin/bash # Description: Extract the implementation of the useAppConfig function and the config object from app/store/config.ts. # Test: Extract the relevant code from app/store/config.ts. rg 'useAppConfig' -A 20 app/store/config.tsLength of output: 565
Script:
#!/bin/bash # Description: Extract the complete implementation of the useAppConfig function from app/store/config.ts. # Test: Extract additional lines from app/store/config.ts to capture the full implementation of the useAppConfig function. rg 'useAppConfig' -A 100 app/store/config.tsLength of output: 1939
Script:
#!/bin/bash # Description: Locate the definition of the createPersistStore function to understand its role in providing the update method. # Test: Search for the definition of the createPersistStore function in TypeScript files. rg 'createPersistStore' --glob '*.ts' --glob '*.tsx'Length of output: 1109
Script:
#!/bin/bash # Description: Extract the implementation of the createPersistStore function from app/utils/store.ts. # Test: Extract the relevant code from app/utils/store.ts. rg 'createPersistStore' -A 50 app/utils/store.tsLength of output: 981
app/components/settings.tsx
Outdated
const availableModelsStr = await accessStore.fetchAvailableModels(accessStore.openaiUrl, accessStore.openaiApiKey); | ||
config.update((config) => (config.customModels = availableModelsStr)); |
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.
Consider using a more descriptive variable name than availableModelsStr
for clarity.
- const availableModelsStr = await accessStore.fetchAvailableModels(accessStore.openaiUrl, accessStore.openaiApiKey);
+ const availableModelIds = await accessStore.fetchAvailableModels(accessStore.openaiUrl, accessStore.openaiApiKey);
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.
const availableModelsStr = await accessStore.fetchAvailableModels(accessStore.openaiUrl, accessStore.openaiApiKey); | |
config.update((config) => (config.customModels = availableModelsStr)); | |
const availableModelIds = await accessStore.fetchAvailableModels(accessStore.openaiUrl, accessStore.openaiApiKey); | |
config.update((config) => (config.customModels = availableModelIds)); |
app/store/access.ts
Outdated
fetchAvailableModels(url: string, apiKey: string): Promise<string>{ | ||
// if (fetchState > 0 || getClientConfig()?.buildMode === "export") | ||
// return Promise.resolve(DEFAULT_ACCESS_STATE.customModels); | ||
|
||
fetchState = 1; | ||
return fetch(url+"/v1/models", { | ||
method: "get", | ||
body: null, | ||
headers: { | ||
"Content-Type": "application/json", | ||
"Authorization": `Bearer ${apiKey}`, | ||
}, | ||
}) | ||
.then((res) => res.json()) | ||
.then((res) => { | ||
// 如果res里的data数组长度大于0,则说明有可用的模型,提取其中的id属性,使用英文逗号进行字符串连接 | ||
if (res.data && res.data.length > 0){ | ||
const excludedKeywords = ['text-', 'moderation', 'embedding', 'dall-e-', 'davinci', 'babbage', 'midjourney', 'whisper', 'tts']; | ||
const availableModels = res.data | ||
.filter((model: any) => !excludedKeywords.some(keyword => model.id.toLowerCase().includes(keyword.toLowerCase()))) | ||
.map((model: any) => model.id) | ||
.join(','); | ||
console.log("availableModels",availableModels); | ||
return `-all,${availableModels}`; | ||
} | ||
else{ | ||
return DEFAULT_ACCESS_STATE.customModels; | ||
} | ||
}) | ||
.catch((error) => { | ||
console.error("[Access] failed to fetch available models: ", error); | ||
return DEFAULT_ACCESS_STATE.customModels; | ||
}) | ||
.finally(() => { | ||
fetchState = 2; | ||
}); | ||
} |
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 fetchAvailableModels
to improve type safety and simplify header.
- fetchAvailableModels(url: string, apiKey: string): Promise<string>{
+ fetchAvailableModels(url: string, apiKey: string): Promise<string> {
- fetchState = 1;
- return fetch(url+"/v1/models", {
- method: "get",
- body: null,
- headers: {
- "Content-Type": "application/json",
- "Authorization": `Bearer ${apiKey}`,
- },
- })
+ if (fetchState !== 0) return Promise.resolve(DEFAULT_ACCESS_STATE.customModels);
+ fetchState = 1;
+ return fetch(`${url}/v1/models`, {
+ method: "GET",
+ headers: {
+ 'Authorization': `Bearer ${apiKey}`
+ }
+ })
- .then((res) => res.json())
- .then((res) => {
- if (res.data && res.data.length > 0){
- const excludedKeywords = ['text-', 'moderation', 'embedding', 'dall-e-', 'davinci', 'babbage', 'midjourney', 'whisper', 'tts'];
- const availableModels = res.data
- .filter((model: any) => !excludedKeywords.some(keyword => model.id.toLowerCase().includes(keyword.toLowerCase())))
- .map((model: any) => model.id)
- .join(',');
- console.log("availableModels",availableModels);
- return `-all,${availableModels}`;
- }
- else{
- return DEFAULT_ACCESS_STATE.customModels;
- }
- })
+ .then(res => res.json())
+ .then(res => {
+ const models = res.data || [];
+ const excludedKeywords = ['text-', 'moderation', 'embedding', 'dall-e-', 'davinci', 'babbage', 'midjourney', 'whisper', 'tts'];
+ const availableModels = models
+ .filter(model => !excludedKeywords.some(keyword => model.id.toLowerCase().includes(keyword.toLowerCase())))
+ .map(model => model.id)
+ .join(',');
+ console.log("availableModels", availableModels);
+ return `-all,${availableModels}`;
+ })
- .catch((error) => {
- console.error("[Access] failed to fetch available models: ", error);
- return DEFAULT_ACCESS_STATE.customModels;
- })
+ .catch(error => {
+ console.error("[Access] failed to fetch available models:", error);
+ return DEFAULT_ACCESS_STATE.customModels;
+ })
- .finally(() => {
- fetchState = 2;
- });
+ .finally(() => fetchState = 2);
- Use simplified header syntax.
- Avoid using
any
type for model filtering. Consider defining a model interface. - Ensure proper handling of fetch states to avoid unnecessary API calls.
Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 133-133: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 142-142: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 145-145: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/components/settings.tsx (1 hunks)
- app/store/access.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/components/settings.tsx
- app/store/access.ts
Summary by CodeRabbit