feat: bot message continuation #5440
Conversation
|
@skymkmk 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 several modifications across multiple files, primarily enhancing the chat functionality. Key changes include updates to the Changes
Possibly related PRs
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: 0
Outside diff range and nitpick comments (2)
app/client/controller.ts (1)
29-31: LGTM! Consider adding error handling for unexpected key formats.The
getPendingMessageIdfunction looks good and provides a useful way to access pending message IDs. The implementation is straightforward and relies on the key format beingsessionId,messageId.Consider adding error handling for cases where the key format is unexpected. This could help catch potential bugs early and make the function more robust.
Here's an example of how you could add error handling:
getPendingMessageId() { - return Object.keys(this.controllers).map((v) => v.split(",").at(-1)); + return Object.keys(this.controllers).map((v) => { + const parts = v.split(","); + if (parts.length !== 2) { + console.warn(`Unexpected key format: ${v}`); + return null; + } + return parts.at(-1); + }).filter((v): v is string => v !== null); },This will log a warning for unexpected key formats and return
nullfor those cases. Thefilterat the end removes anynullvalues from the resulting array.app/store/chat.ts (1)
477-487: The tool update logic looks fine, but let's address the static analysis hint.The code segment correctly updates the corresponding tool in the
botMessage.toolsarray with the latest information provided by thetoolparameter. It iterates over thetoolsarray, finds the matching tool based on theid, and updates its properties using the spread operator.However, the static analysis tool suggests that the assignment should not be in an expression. To improve clarity and avoid potential confusion, consider extracting the assignment into a separate statement:
botMessage?.tools?.forEach((t, i, tools) => { if (tool.id == t.id) { - tools[i] = { ...tool }; + const updatedTool = { ...tool }; + tools[i] = updatedTool; } });This change makes the assignment explicit and separates it from the expression.
Tools
Biome
[error] 477-477: 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)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
app/icons/continue.svgis excluded by!**/*.svg
Files selected for processing (27)
- app/client/api.ts (1 hunks)
- app/client/controller.ts (1 hunks)
- app/client/platforms/anthropic.ts (1 hunks)
- app/client/platforms/moonshot.ts (1 hunks)
- app/client/platforms/openai.ts (2 hunks)
- app/components/chat.tsx (5 hunks)
- app/locales/ar.ts (1 hunks)
- app/locales/bn.ts (1 hunks)
- app/locales/cn.ts (1 hunks)
- app/locales/cs.ts (1 hunks)
- app/locales/de.ts (1 hunks)
- app/locales/en.ts (1 hunks)
- app/locales/es.ts (1 hunks)
- app/locales/fr.ts (1 hunks)
- app/locales/id.ts (1 hunks)
- app/locales/it.ts (1 hunks)
- app/locales/jp.ts (1 hunks)
- app/locales/ko.ts (1 hunks)
- app/locales/no.ts (1 hunks)
- app/locales/pt.ts (1 hunks)
- app/locales/ru.ts (1 hunks)
- app/locales/sk.ts (1 hunks)
- app/locales/tr.ts (1 hunks)
- app/locales/tw.ts (1 hunks)
- app/locales/vi.ts (1 hunks)
- app/store/chat.ts (4 hunks)
- app/utils/chat.ts (4 hunks)
Files skipped from review due to trivial changes (16)
- app/locales/ar.ts
- app/locales/bn.ts
- app/locales/cn.ts
- app/locales/cs.ts
- app/locales/de.ts
- app/locales/en.ts
- app/locales/es.ts
- app/locales/fr.ts
- app/locales/it.ts
- app/locales/jp.ts
- app/locales/ko.ts
- app/locales/no.ts
- app/locales/pt.ts
- app/locales/ru.ts
- app/locales/tr.ts
- app/locales/vi.ts
Additional context used
Biome
app/store/chat.ts
[error] 477-477: 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)
app/components/chat.tsx
[error] 470-472: 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] 1060-1063: 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] 1193-1196: 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 not posted (19)
app/client/platforms/moonshot.ts (1)
166-166: Review and harmonizeparseSSEimplementations across filesThe
parseSSEfunction inapp/utils/chat.tsalready returns an object with bothdeltaandfinishReasonproperties. This implementation is more flexible than the proposed change to{ delta: string }. The usage ofparseSSEin this file also handles both string and object return types.However, there seem to be multiple implementations of
parseSSEacross different files:
- app/utils/chat.ts
- app/client/platforms/anthropic.ts
- app/client/platforms/openai.ts
- app/client/platforms/moonshot.ts
To ensure consistency and prevent potential issues:
- Review all
parseSSEimplementations to ensure they return the same type.- Consider standardizing on the more flexible return type:
{ delta?: string; finishReason?: string }.- Verify that all callers of
parseSSEhandle the object return type correctly.- Maintain the flexible type checking in
app/utils/chat.tsas it provides better backwards compatibility.Analysis chain
Verify the impact of the return type change.
The change in the return type of the
parseSSEfunction fromstringto{ delta: string }is logically correct. However, it's important to ensure that all the callers of this function are updated to handle the new object format instead of expecting a direct string value.Run the following script to verify the usage of the
parseSSEfunction:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all callers of the `parseSSE` function are updated to handle the new return type. # Test: Search for the function usage. Expect: Callers should access the `delta` property of the returned object. rg --type typescript -A 5 $'parseSSE'Length of output: 71
Script:
#!/bin/bash # Description: Verify all callers of the `parseSSE` function are updated to handle the new return type. # Search for .ts files containing 'parseSSE' and show 5 lines after the match echo "Searching with ripgrep:" rg -t ts -A 5 'parseSSE' echo -e "\nSearching with fd and grep:" fd -e ts -x grep -n -A 5 'parseSSE' {} echo -e "\nSearching with ast-grep:" ast-grep --lang typescript --pattern 'parseSSE($_)'Length of output: 4249
app/client/api.ts (1)
62-62: LGTM!The addition of the optional
finishedReasonparameter to theonFinishcallback enhances the flexibility and detail in handling chat completion events. This change is backwards compatible, as the new parameter is optional, ensuring existing implementations of theonFinishcallback will continue to work without modification.Developers can choose to leverage the new parameter to implement more nuanced logic based on the reason for the completion, such as handling user actions, errors, or natural conclusions of the conversation differently.
app/utils/chat.ts (4)
172-172: LGTM!Explicitly typing the
optionsparameter asChatOptionsimproves code readability, maintainability, and type safety. It helps catch type-related issues at compile-time and provides better IDE support for the available options.
177-177: LGTM!The introduction of the
finishedReasonvariable aligns with the changes made to theparseSSEfunction return type. It allows for storing and utilizing the reason for the stream completion, providing more context about the completion status.
339-343: LGTM!The updated handling of the
chunkreturned fromparseSSEaligns with the changes made to the function's return type. By checking the type ofchunkand handling thedeltaandfinishReasonproperties accordingly, the code ensures that the message content is accumulated correctly and the completion reason is stored for later use.
163-166: Verify theparseSSEfunction usage across the codebase.The updated return type of the
parseSSEfunction provides more granular information about the server-sent events (SSE) response, which is a positive change. It allows for better handling of the streaming response and provides more context about the completion status.Please ensure that all the code segments consuming the response from
parseSSEhave been updated to handle the new return type. You can use the following script to verify the usage:Verification successful
parseSSE function usage verified and updated across the codebase
The verification process confirms that all occurrences of the
parseSSEfunction have been updated to use the new return type{ delta?: string; finishReason?: string }. No instances of the old usage pattern (returning just a string) were found.However, multiple implementations or usages of
parseSSEwere detected in different contexts. While this doesn't affect the correctness of the current changes, it might be worth considering:
- Consolidating these implementations if they serve the same purpose to improve maintainability.
- Clearly documenting the differences if these are intentionally separate implementations for different use cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of the `parseSSE` function have been updated to handle the new return type. # Test: Search for the function usage. Expect: No occurrences of the old usage. rg --type typescript -A 5 $'parseSSE\\(.*\\):\\s*string'Length of output: 89
Script:
#!/bin/bash # Description: Verify all usages of the `parseSSE` function have been updated to handle the new return type. # Test 1: Search for the old function usage in .ts files echo "Searching for old parseSSE usage in .ts files:" fd -e ts -x rg -A 5 'parseSSE\(.*\):\s*string' {} # Test 2: Search for the old function usage in .tsx files echo "Searching for old parseSSE usage in .tsx files:" fd -e tsx -x rg -A 5 'parseSSE\(.*\):\s*string' {} # Test 3: Search for all occurrences of parseSSE to verify its usage echo "Searching for all parseSSE occurrences:" fd -e ts -e tsx -x rg -A 5 'parseSSE' {}Length of output: 1840
app/client/platforms/anthropic.ts (1)
265-265: LGTM! Verify the impact on the consuming code.The change in the return value format from a string to an object allows for future extensibility. However, ensure that the code consuming the output of this function is updated to handle the new format.
To verify the impact on the consuming code, run the following script:
Verification successful
Verification successful: The change is safe and properly handled.
The usage of
parseSSEinapp/utils/chat.tscorrectly handles both the old (string) and new (object) return value formats. The code checks for both string and object withdeltaproperty, ensuring backward compatibility while supporting the new format. There are no other active usages ofparseSSEin the codebase that need to be updated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `parseSSE` function and its return value. # Test: Search for the usage of the `parseSSE` function. Expect: The consuming code handles the new return value format. rg --type typescript -A 5 $'parseSSE'Length of output: 71
Script:
#!/bin/bash # Description: Verify the existence of TypeScript files and search for parseSSE usage # Check for TypeScript files echo "Checking for TypeScript files:" fd -e ts # Search for parseSSE definition and usage echo -e "\nSearching for parseSSE definition and usage:" rg -n "parseSSE" --type-add 'ts:*.ts' --type tsLength of output: 2770
Script:
#!/bin/bash # Description: Examine parseSSE usage and search for additional active usages # Check the usage of parseSSE in app/utils/chat.ts echo "Examining parseSSE usage in app/utils/chat.ts:" sed -n '/const chunk = parseSSE/,+5p' app/utils/chat.ts # Search for any other active (non-commented) usages of parseSSE echo -e "\nSearching for other active parseSSE usages:" rg -n "parseSSE\(" --type-add 'ts:*.ts' --type ts | grep -v "\/\/"Length of output: 677
app/locales/tw.ts (1)
48-48: LGTM!The addition of the
"Continue": "繼續"key-value pair in theMessagesobject is a valid and helpful enhancement to the Traditional Chinese localization.app/client/platforms/openai.ts (2)
269-269: LGTM!The addition of the optional
finish_reasonproperty to thechoicesarray in the response structure is a valid enhancement. It provides more detailed information about the completion status of the API response, which can be useful for handling different response scenarios, such as message continuation.
290-293: Looks good!The modification to the return value of the method that processes the API response is a logical change. Returning an object containing both the
delta(content) and thefinishReasonproperties provides a more structured way to access the response data. This change aligns with the addition of thefinish_reasonproperty to the response structure.app/locales/sk.ts (1)
50-50: LGTM!The addition of the Slovak translation for "Continue" is correctly formatted and enhances the user interface for Slovak-speaking users.
app/locales/id.ts (1)
48-48: LGTM!The addition of the
Continuekey and its Indonesian translation"Lanjutkan"is consistent with the existing localization file structure and style.app/store/chat.ts (5)
49-49: LGTM!The new optional property
finishedReasonis a clear and appropriate addition to theChatMessageinterface. It won't break existing code.
377-380: Looks good!The
onFinishmethod is updated appropriately to handle the new optionalfinishedReasonparameter and store it in thebotMessageobject when provided.
435-521: Great work on implementing the message continuation feature!The
onContinueBotMessagemethod is well-structured and handles the continuation process effectively. It retrieves the necessary messages, finds the correspondingbotMessage, and makes an API request with appropriate callbacks to handle updates, completion, tool usage, errors, and controller management.The method follows a logical flow and includes error handling for cases when the
botMessageis not found.Overall, the implementation looks solid and adds a valuable feature to the chat functionality.
Tools
Biome
[error] 477-477: 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)
535-545: The updates togetMessagesWithMemorylook good!The changes allow the method to accept an optional
messageIDparameter and retrieve messages based on the provided ID. It finds the index of the message with the matching ID and sets the appropriate starting point for message retrieval.If the message is not found, it handles the case by considering all messages. The calculation of
totalMessageCountensures the correct number of messages are retrieved based on themessageIdxand the length ofsession.messages.These updates enhance the flexibility of the
getMessagesWithMemorymethod and support the message continuation feature.
492-511: The error handling logic in theonErrorcallback looks solid.The code segment appropriately handles errors that occur during the continuation process. It checks if the error is due to an aborted request by looking for the "aborted" keyword in the error message.
It appends the error details to the
botMessage.contentproperty to inform the user about the error. SettingbotMessage.streamingtofalsecorrectly indicates that the streaming has stopped, and settingbotMessage.isErrorbased on the nature of the error provides additional information.Updating the
session.messagesarray with the modifiedbotMessageensures that the UI reflects the error state. Removing the associated controller usingChatControllerPool.removeis a good practice to clean up resources and prevent further actions on the aborted or failed continuation.Logging the error to the console is helpful for debugging and monitoring purposes.
Overall, the error handling logic is comprehensive and handles different scenarios appropriately.
app/components/chat.tsx (2)
465-474: LGTM!The
stopAllfunction correctly stops all pending messages and updates the session state by marking the stopped messages as "aborted".Regarding the static analysis hint about the assignment in the expression at lines 470-472:
This is a common and idiomatic pattern in JavaScript for mapping over an array and conditionally updating elements based on a condition. It does not introduce any bugs or confusion in this context. The static analysis tool has raised a false positive here which can be safely ignored.
Tools
Biome
[error] 470-472: 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)
1190-1200: Looks good!The
onContinuefunction properly updates the session state to mark the message as streaming and triggers the continuation of the bot's response by callingchatStore.onContinueBotMessagewith the message ID.Regarding the static analysis hint about the assignment in the expression at lines 1193-1196:
As mentioned in the previous comment, this is a common and acceptable pattern in JavaScript for conditionally updating array elements during mapping. The static analysis tool has incorrectly flagged this as an issue. It's safe to disregard this hint.
Tools
Biome
[error] 1193-1196: 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)
|
Your build has completed! |
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
为因达到 max_token 限制（Currently only available for OpenAI API）和手动停止的 assistant 消息提供继续生成的功能
📝 补充信息 | Additional Information
#5439
对于 GPT 与 Claude，当发送的 message 最后角色为 assistant 时，模型会尝试继续回复。(但因我没有 Claude API 用来测试结束原因，所以只完成了 OpenAI API 的功能)
由于网络故障会导致消息内容污染，进而很大概率导致模型直接重新生成消息，故未根据
isError判断是否提供继续按钮。应当等待后续将 message 与 error 消息分开再行添加。Summary by CodeRabbit
Release Notes
New Features
Localization
Bug Fixes
These updates enhance user experience, providing greater control and flexibility within the chat interface.