修复自定义模型与预设模型名重复时,无法正确将自定义模型认定为openai渠道的bug #4748
Conversation
|
@imaiya is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
|
Your build has completed! |
|
Fixes #4492 |
Fixes #4492 |
WalkthroughThe recent updates primarily focus on enhancing various aspects of the application's user interface and backend logic. The Changes
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 Configration 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: 0
Outside diff range and nitpick comments (2)
app/utils/model.ts (2)
Line range hint
25-30: Consider replacingforEachwithfor...ofloops for better readability and potential performance benefits in modern JavaScript engines.- models.forEach((m) => { + for (const m of models) {Also applies to: 33-55, 44-46
Line range hint
65-65: Change theletdeclaration toconstsince the variablemodelTableis not reassigned, which can help ensure immutability and reduce potential bugs.- let modelTable = collectModelTable(models, customModels); + const modelTable = collectModelTable(models, customModels);
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/utils/model.ts (2 hunks)
Additional Context Used
Biome (6)
app/utils/model.ts (6)
25-30: Prefer for...of instead of forEach.
33-55: Prefer for...of instead of forEach.
44-46: Prefer for...of instead of forEach.
45-45: The assignment should not be in an expression.
1-1: All these imports are only used as types.
65-65: This let declares a variable that is only assigned once.
Additional comments not posted (2)
app/utils/model.ts (2)
5-5: The addition ofproviderNameandproviderTypeincustomProvideraligns with the PR objectives to distinguish custom models as OpenAI channel models. TheproviderTypefield, although not mentioned in the summary, is a logical addition for further categorization.
52-52: Simplifying the provider assignment incollectModelTableto always usecustomProviderensures that custom models are prioritized correctly, aligning with the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (6)
app/components/sidebar.tsx (2)
Line range hint
191-198: Add keyboard event handlers to improve accessibility.+ onKeyUp={handleKeyUp}Ensure to implement
handleKeyUpto handle keyboard interactions for better accessibility.
Line range hint
117-117: Includelimitin the dependency array to ensure the hook updates correctly.- }, [config.sidebarWidth, isMobileScreen, shouldNarrow]); + }, [config.sidebarWidth, isMobileScreen, shouldNarrow, limit]);app/components/chat.tsx (4)
382-382: Clarify the use of data attributes for style control.Using data attributes (
data-expanded) for controlling styles is unusual and might not be clear to all developers. Consider adding a comment explaining this choice or using a more conventional method like conditional class names.
559-559: Ensure consistency in the use of boolean props.The
isExpandedprop is explicitly set totruehere, which is fine, but ensure that it is consistently handled across all uses of theChatActioncomponent to avoid unexpected behaviors.
Line range hint
257-261: Refactor theuseEffectto correctly specify dependencies.The
useEffecthook does not correctly specify all its dependencies, which could lead to bugs due to missing updates.useEffect(() => { setSelectIndex(0); - }, [props.prompts.length]); + }, [props.prompts.length, noPrompts, props.onPromptSelect, props.prompts.at]);
Line range hint
474-475: Include all necessary dependencies in theuseEffecthook.The
useEffecthook used for showing the upload image button does not include all necessary dependencies, which might lead to unexpected behavior.useEffect(() => { const show = isVisionModel(currentModel); setShowUploadImage(show); if (!show) { props.setAttachImages([]); props.setUploading(false); } - }, [chatStore, currentModel, models]); + }, [chatStore, currentModel, models, props.setAttachImages, props.setUploading]);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- app/components/chat.module.scss (3 hunks)
- app/components/chat.tsx (5 hunks)
- app/components/sidebar.tsx (1 hunks)
- app/constant.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- app/constant.ts
Additional context used
Biome
app/components/sidebar.tsx
[error] 191-198: 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] 117-117: This hook does not specify all of its dependencies: limit (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
app/components/chat.tsx
[error] 182-186: 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] 302-311: 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] 324-331: 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] 368-384: 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] 385-385: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 387-387: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 668-669: 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] 1522-1523: Avoid the autoFocus attribute. (lint/a11y/noAutofocus)
Unsafe fix: Remove the autoFocus attribute.
[error] 257-257: This hook specifies more dependencies than necessary: props.prompts.length (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
[error] 261-261: This hook does not specify all of its dependencies: noPrompts (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 261-261: This hook does not specify all of its dependencies: props.onPromptSelect (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 261-261: This hook does not specify all of its dependencies: props.prompts.at (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 364-364: This hook does not specify all of its dependencies: updateWidth (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 364-364: This hook specifies more dependencies than necessary: props.icon, props.text (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
This dependency can be removed from the list.
[error] 474-475: This hook does not specify all of its dependencies: props.setAttachImages (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 474-475: This hook does not specify all of its dependencies: props.setUploading (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 736-736: This hook specifies more dependencies than necessary: userInput (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
[error] 814-815: This hook does not specify all of its dependencies: chatStore.updateCurrentSession (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
[error] 814-815: This hook does not specify all of its dependencies: config.modelConfig (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
[error] 1114-1114: This hook does not specify all of its dependencies: session.id (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
Additional comments not posted (2)
app/components/sidebar.tsx (1)
161-161: Text change approved. It enhances clarity by specifying the author.app/components/chat.module.scss (1)
90-96: CSS changes approved. They ensure that elements remain expanded as intended.
| @@ -341,6 +341,7 @@ function ChatAction(props: { | |||
| text: string; | |||
| icon: JSX.Element; | |||
| onClick: () => void; | |||
| isExpanded?: boolean; | |||
There was a problem hiding this comment.
Add keyboard accessibility to the ChatAction component.
The ChatAction component should also respond to keyboard events to ensure accessibility. Consider adding onKeyDown or similar keyboard event handlers.
function ChatAction(props: {
text: string;
icon: JSX.Element;
onClick: () => void;
isExpanded?: boolean;
}) {
+ onKeyDown={(e) => e.key === 'Enter' && props.onClick()}
...
}Committable suggestion was skipped due to low confidence.
| useEffect(() => { | ||
| updateWidth(); | ||
| }, [props.icon, props.text]); |
There was a problem hiding this comment.
Ensure updateWidth is included in the dependency array of useEffect.
The updateWidth function is used inside a useEffect but is not included in its dependency array. This might lead to stale closures capturing outdated values.
useEffect(() => {
updateWidth();
- }, [props.icon, props.text]);
+ }, [props.icon, props.text, updateWidth]);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.
| useEffect(() => { | |
| updateWidth(); | |
| }, [props.icon, props.text]); | |
| useEffect(() => { | |
| updateWidth(); | |
| }, [props.icon, props.text, updateWidth]); |
Tools
Biome
[error] 364-364: This hook does not specify all of its dependencies: updateWidth (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 364-364: This hook specifies more dependencies than necessary: props.icon, props.text (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
This dependency can be removed from the list.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/constant.ts (1 hunks)
Additional comments not posted (1)
app/constant.ts (1)
1-1: Update OWNER constant to reflect current maintainer.The change from "Yidadaa" to "imaiya" aligns with the PR's objective to recognize contributions and ownership accurately. This update should reflect across all associated URLs and configurations that utilize the
OWNERconstant.
- 将 sendPreviewBubble 设置为 false，以禁用预览气泡。 - 将 dontShowMaskSplashScreen 设置为 true，以隐藏创建聊天时的闪屏。
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
app/store/config.ts (1)
Line range hint
74-74: ReplaceisNaNwithNumber.isNaNto avoid type coercion.The use of
isNaNcan lead to unexpected behavior due to type coercion. It's safer and more predictable to useNumber.isNaNwhich does not coerce the type.- if (isNaN(x)) { + if (Number.isNaN(x)) {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/store/config.ts (1 hunks)
Additional context used
Biome
app/store/config.ts
[error] 74-74: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
Additional comments not posted (1)
app/store/config.ts (1)
37-37: Consider the implications of toggling configuration flags.The change in
sendPreviewBubblefromtruetofalseanddontShowMaskSplashScreenfromfalsetotruemay have significant implications on the user interface and user experience. Ensure that these changes align with the desired behavior in the application's settings and that they have been thoroughly tested.Also applies to: 43-43
|
|
Summary by CodeRabbit
New Features
ChatActionprop for expanded state.Bug Fixes
Style
Chores