-
Notifications
You must be signed in to change notification settings - Fork 90
feat(demo): add chat-x demo #44
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
Conversation
WalkthroughThe pull request introduces various enhancements to the Changes
Possibly related PRs
Suggested reviewers
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 (
|
6dd56e2 to
73738ad
Compare
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (5)
examples/coze-js-web/src/pages/chat-x/index.tsx (3)
133-133: Adjust the error message to reflect required configuration parametersThe error message mentions 'voice file ID', but this component seems not to require a 'voice file ID'. Consider removing 'voice file ID' from the message to avoid confusion.
72-77: Clarify parameter naming in the request functionIn the
requestfunction, the parameter destructuring{ message: query }renamesmessagetoquery. This can be confusing. Consider using consistent parameter names to improve readability.
147-150: Improve clarity of error message when adding new conversationsThe error message "Don't create more than one empty conversation" might be confusing to users. Consider rephrasing it to guide users more effectively, such as "Please complete the existing new conversation before starting another one."
examples/coze-js-web/src/pages/chat-x/settings.tsx (1)
26-27: Avoid using 'any' type for function parametersUsing
anydefeats the purpose of TypeScript's type checking. Consider defining a specific type forvalues, or useunknownand perform type assertions.Apply this diff to specify the type and remove the ESLint disable comment:
-// eslint-disable-next-line @typescript-eslint/no-explicit-any -const handleSettingsSave = (values: any) => { +interface SettingsValues { + base_url: string; + pat: string; + bot_id: string; +} + +const handleSettingsSave = (values: SettingsValues) => {examples/coze-js-web/src/pages/chat-x/use-coze-api.ts (1)
128-136: Cache bot info to prevent unnecessary API callsThe
getBotInfofunction makes an API call on every invocation. Consider caching the result.+const BOT_INFO_CACHE_TIME = 5 * 60 * 1000; // 5 minutes + const getBotInfo = async () => { if (!clientRef.current) { return; } + + // Return cached info if available and not expired + if (botInfo && botInfo.lastFetched && Date.now() - botInfo.lastFetched < BOT_INFO_CACHE_TIME) { + return botInfo; + } + const res = await clientRef.current.bots.retrieve({ bot_id: config.getBotId(), }); - setBotInfo(res); + setBotInfo({ ...res, lastFetched: Date.now() }); };
🛑 Comments failed to post (6)
examples/coze-js-web/src/pages/chat-x/index.tsx (1)
184-186:
⚠️ Potential issueEnsure file list is not empty before accessing
Currently, the code assumes that
info.fileList[0]exists, which might cause an error if the file list is empty. Consider adding a check forinfo.fileList.length > 0before accessinginfo.fileList[0].Apply this diff to fix the issue:
const handleFileChange: GetProp<typeof Attachments, 'onChange'> = info => { setAttachedFiles(info.fileList); + if (info.fileList.length > 0) { uploadFile(info.fileList[0].originFileObj as File); + } };📝 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.setAttachedFiles(info.fileList); if (info.fileList.length > 0) { uploadFile(info.fileList[0].originFileObj as File); }examples/coze-js-web/package.json (1)
29-30:
⚠️ Potential issueFix version mismatch between @types/lodash and lodash
The @types/lodash version (~4.17.13) doesn't match the lodash version (~4.17.21). This could lead to type definition inconsistencies.
Update the version to match:
- "@types/lodash": "~4.17.13", + "@types/lodash": "~4.17.21",📝 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."@types/lodash": "~4.17.21", "@types/react-router-dom": "~5.3.3",examples/coze-js-web/src/pages/chat-x/use-coze-api.ts (4)
21-29:
⚠️ Potential issueAdd error handling for invalid configuration
The
initClientfunction doesn't validate the presence of required configuration values or handle potential initialization errors.Add validation and error handling:
const initClient = () => { const baseUrl = config.getBaseUrl(); const pat = config.getPat(); + if (!baseUrl || !pat) { + throw new Error('Missing required configuration: baseUrl and pat must be provided'); + } clientRef.current = new CozeAPI({ token: pat, baseURL: baseUrl, allowPersonalAccessTokenInBrowser: true, }); };📝 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.const initClient = () => { const baseUrl = config.getBaseUrl(); const pat = config.getPat(); if (!baseUrl || !pat) { throw new Error('Missing required configuration: baseUrl and pat must be provided'); } clientRef.current = new CozeAPI({ token: pat, baseURL: baseUrl, allowPersonalAccessTokenInBrowser: true, }); };
31-116:
⚠️ Potential issueImprove error handling and add request timeout for streaming chat
The streaming chat implementation needs better error handling for network issues and timeouts.
Consider these improvements:
- Add timeout handling
- Add error handling for network issues
- Clean up resources on error
const streamingChat = async ({ query, conversationId, onUpdate, onSuccess, onCreated, + timeout = 30000, }: { query: string; conversationId?: string; onUpdate: (delta: string) => void; onSuccess: (delta: string) => void; onCreated: (data: CreateChatData) => void; + timeout?: number; }) => { if (!clientRef.current) { - return; + throw new Error('Client not initialized'); } + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), timeout); + try { const botId = config.getBotId(); + if (!botId) { + throw new Error('Bot ID not configured'); + } // ... rest of the implementation ... + } catch (error) { + console.error('Streaming chat error:', error); + throw error; + } finally { + clearTimeout(timeoutId); + } };Committable suggestion skipped: line range outside the PR's diff.
118-126: 🛠️ Refactor suggestion
Add file validation and progress tracking for uploads
The file upload implementation should validate file types/sizes and track upload progress.
const uploadFile = useCallback(async (file: File) => { if (!clientRef.current) { throw new Error('Client not initialized'); } + + // Validate file size and type + const MAX_SIZE = 50 * 1024 * 1024; // 50MB + if (file.size > MAX_SIZE) { + throw new Error('File size exceeds 50MB limit'); + } + + // Add progress tracking const res = await clientRef.current.files.upload({ file, + onProgress: (event) => { + const progress = (event.loaded / event.total) * 100; + console.log(`Upload progress: ${progress}%`); + } }); fileInfoRef.current = res; }, []);Committable suggestion skipped: line range outside the PR's diff.
1-148: 🛠️ Refactor suggestion
Consider implementing cleanup and resource management
The hook should clean up resources when unmounted.
Add cleanup logic using useEffect:
+import { useCallback, useRef, useState, useEffect } from 'react'; const useCozeAPI = () => { // ... existing implementation ... + + useEffect(() => { + return () => { + // Clean up resources + clientRef.current = null; + fileInfoRef.current = null; + }; + }, []); + return { // ... existing return ... }; };📝 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.import { useCallback, useRef, useState, useEffect } from 'react'; import { type BotInfo, ChatEventType, CozeAPI, type CreateChatData, type EnterMessage, type FileObject, RoleType, } from '@coze/api'; import { config } from './config'; const useCozeAPI = () => { const clientRef = useRef<CozeAPI | null>(null); const [botInfo, setBotInfo] = useState<BotInfo | null>(null); const fileInfoRef = useRef<FileObject | null>(null); const initClient = () => { const baseUrl = config.getBaseUrl(); const pat = config.getPat(); clientRef.current = new CozeAPI({ token: pat, baseURL: baseUrl, allowPersonalAccessTokenInBrowser: true, }); }; const streamingChat = async ({ query, conversationId, onUpdate, onSuccess, onCreated, }: { query: string; conversationId?: string; onUpdate: (delta: string) => void; onSuccess: (delta: string) => void; onCreated: (data: CreateChatData) => void; }) => { if (!clientRef.current) { return; } const botId = config.getBotId(); let messages: EnterMessage[] = []; if (fileInfoRef.current) { messages = [ { role: RoleType.User, type: 'question', content: [ { type: 'text', text: query, }, { type: 'file', file_id: fileInfoRef.current.id, }, ], content_type: 'object_string', }, ]; } else { messages = [ { role: RoleType.User, type: 'question', content: [ { type: 'text', text: query, }, ], content_type: 'text', }, ]; } const v = await clientRef.current.chat.stream({ bot_id: botId, auto_save_history: true, additional_messages: messages, conversation_id: conversationId, }); let msg = ''; for await (const part of v) { if (part.event === ChatEventType.CONVERSATION_CHAT_CREATED) { console.log('[START]'); onCreated(part.data); } else if (part.event === ChatEventType.CONVERSATION_MESSAGE_DELTA) { msg += part.data.content; onUpdate(msg); } else if (part.event === ChatEventType.CONVERSATION_MESSAGE_COMPLETED) { const { role, type, content: msgContent } = part.data; if (role === 'assistant' && type === 'answer') { msg += '\n'; onSuccess(msg); } else { console.log('[%s]:[%s]:%s', role, type, msgContent); } } else if (part.event === ChatEventType.CONVERSATION_CHAT_COMPLETED) { console.log(part.data.usage); } else if (part.event === ChatEventType.DONE) { console.log(part.data); } } console.log('=== End of Streaming Chat ==='); }; const uploadFile = useCallback(async (file: File) => { if (!clientRef.current) { throw new Error('Client not initialized'); } const res = await clientRef.current.files.upload({ file, }); fileInfoRef.current = res; }, []); const getBotInfo = async () => { if (!clientRef.current) { return; } const res = await clientRef.current.bots.retrieve({ bot_id: config.getBotId(), }); setBotInfo(res); }; useEffect(() => { return () => { // Clean up resources clientRef.current = null; fileInfoRef.current = null; }; }, []); return { client: clientRef.current, initClient, streamingChat, uploadFile, getBotInfo, botInfo, }; }; export default useCozeAPI;
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: 5
🧹 Outside diff range and nitpick comments (1)
examples/coze-js-web/src/pages/chat-x/use-coze-api.ts (1)
15-20: Consider clearing fileInfoRef after useThe
fileInfoRefmaintains the last uploaded file information indefinitely. This could lead to unexpected behavior where a previous file is included in new messages if not explicitly cleared.Consider adding a method to clear the file reference after it's used in a message:
+ const clearFileInfo = useCallback(() => { + fileInfoRef.current = null; + }, []);And call it after the message is sent in the streamingChat function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlexamples/coze-js-web/src/pages/chat-x/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
examples/coze-js-web/package.json(2 hunks)examples/coze-js-web/src/App.tsx(3 hunks)examples/coze-js-web/src/pages/chat-x/config.ts(1 hunks)examples/coze-js-web/src/pages/chat-x/index.tsx(1 hunks)examples/coze-js-web/src/pages/chat-x/settings.tsx(1 hunks)examples/coze-js-web/src/pages/chat-x/use-coze-api.ts(1 hunks)examples/coze-js-web/src/pages/chat-x/use-style.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- examples/coze-js-web/src/App.tsx
- examples/coze-js-web/package.json
- examples/coze-js-web/src/pages/chat-x/use-style.ts
- examples/coze-js-web/src/pages/chat-x/config.ts
- examples/coze-js-web/src/pages/chat-x/settings.tsx
- examples/coze-js-web/src/pages/chat-x/index.tsx
🧰 Additional context used
🪛 GitHub Check: Node.js v20 (ubuntu-latest)
examples/coze-js-web/src/pages/chat-x/use-coze-api.ts
[failure] 11-11: TS Type Check
Cannot find module '@coze/api' or its corresponding type declarations.
🔇 Additional comments (1)
examples/coze-js-web/src/pages/chat-x/use-coze-api.ts (1)
138-148: LGTM: Hook exports are well-structured
The hook exports provide a clean interface with all necessary functions and state.
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: 1
🧹 Outside diff range and nitpick comments (3)
examples/coze-js-web/src/pages/chat-x/index.tsx (3)
131-142: Enhance configuration error messagingThe error message could be more specific about which configuration items are missing.
- message.error('Please set the base URL, PAT, bot ID and voice file ID'); + const missingConfig = []; + if (!baseUrl) missingConfig.push('base URL'); + if (!pat) missingConfig.push('PAT'); + if (!botId) missingConfig.push('bot ID'); + message.error(`Missing configuration: ${missingConfig.join(', ')}`);
153-188: Add error handling for conversation management edge casesThe conversation management logic should handle potential edge cases:
- Maximum number of conversations
- Message map size limits
- Invalid conversation keys
const onAddConversation = () => { + const MAX_CONVERSATIONS = 50; + if (conversationsItems.length >= MAX_CONVERSATIONS) { + message.error('Maximum number of conversations reached'); + return; + } if (conversationsItems.find(item => item.key === '0')) { message.error("Don't create more than one empty conversation"); return; }
294-294: Translate Chinese comment to EnglishFor consistency and maintainability, translate the Chinese comment "消息列表" to English (e.g., "Message List").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
examples/coze-js-web/src/pages/chat-x/index.tsx(1 hunks)examples/coze-js-web/src/pages/chat-x/use-coze-api.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Node.js v20 (ubuntu-latest)
examples/coze-js-web/src/pages/chat-x/use-coze-api.ts
[failure] 11-11: TS Type Check
Cannot find module '@coze/api' or its corresponding type declarations.
examples/coze-js-web/src/pages/chat-x/index.tsx
[failure] 4-4: TS Type Check
Cannot find module '@coze/api' or its corresponding type declarations.
🔇 Additional comments (5)
examples/coze-js-web/src/pages/chat-x/use-coze-api.ts (5)
22-30: Security: Review personal access token usage in browser
Setting allowPersonalAccessTokenInBrowser: true could expose sensitive credentials in the browser environment. This is generally not recommended for production applications.
32-61: LGTM! Well-structured message creation logic
The function properly handles both text-only and file attachment scenarios with appropriate type safety.
117-134: Add file validation and error handling to upload function
The file upload implementation could be enhanced with file validation and better error handling.
136-144: Improve error handling in getBotInfo
The function silently returns if the client is not initialized, which could lead to confusion about why bot info isn't being set.
63-115: 🛠️ Refactor suggestion
Remove console.logs and enhance error handling
The streaming chat implementation needs improvement:
- Remove console.log statements (lines 94, 106, 109, 111, 114)
- Add proper error handling for stream processing
const streamingChat = async ({...}) => {
if (!clientRef.current) {
- return;
+ throw new Error('Client not initialized');
}
try {
const v = await clientRef.current.chat.stream({...});
let msg = '';
for await (const part of v) {
if (part.event === ChatEventType.CONVERSATION_CHAT_CREATED) {
- console.log('[START]');
onCreated(part.data);
} else if (part.event === ChatEventType.CONVERSATION_MESSAGE_DELTA) {
msg += part.data.content;
onUpdate(msg);
} else if (part.event === ChatEventType.CONVERSATION_MESSAGE_COMPLETED) {
const { role, type, content: msgContent } = part.data;
if (role === 'assistant' && type === 'answer') {
msg += '\n';
onSuccess(msg);
- } else {
- console.log('[%s]:[%s]:%s', role, type, msgContent);
}
} else if (part.event === ChatEventType.CONVERSATION_CHAT_COMPLETED) {
- console.log(part.data.usage);
} else if (part.event === ChatEventType.DONE) {
- console.log(part.data);
}
}
- console.log('=== End of Streaming Chat ===');
+ } catch (error) {
+ console.error('Error in streaming chat:', error);
+ throw error;
}
};Likely invalid or redundant comment.
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
🧹 Outside diff range and nitpick comments (4)
packages/coze-js/src/resources/chat/chat.ts (4)
305-312: Consider improving documentation readabilityThe multi-line comments for
additional_messagescould be more concise and better formatted. Consider using bullet points consistently and reducing redundancy.- * Additional information for the conversation. You can pass the user's question in this field. The array length is limited to 100, allowing up to 100 messages. - * - * - When auto_save_history=true, additional_messages will be added to the session as messages first, then passed to the model as context. - * - When auto_save_history=false, additional_messages will only be passed to the model as additional information, and neither additional_messages nor any messages returned by the model will be added to the session. - * - * To ensure model effectiveness, the last message in additional_messages will be passed to the model as the user's input for this conversation, so it is recommended to pass a record with role=user for the last message to avoid affecting the model's performance. - * - * If no session is specified for this conversation or if there are no messages in the specified session, the user's question must be passed through this parameter. + * Additional information for the conversation (max 100 messages): + * • With auto_save_history=true: Messages are added to session and used as context + * • With auto_save_history=false: Messages are only used as context + * • Last message should have role=user for optimal model performance + * • Required if no session exists or has no messages
Line range hint
156-164: Enhance error messages for better debuggingThe error messages in the JSON parsing catch blocks could be more informative by including the operation context and suggesting potential fixes.
- throw new CozeError( - `Could not parse message into JSON:${message.data}`, - ); + throw new CozeError( + `Failed to parse streaming message: ${message.data}. Ensure the response is valid JSON and matches the expected format.`, + );Also applies to: 249-257
Line range hint
746-761: Consider consolidating related type definitionsThe content type definitions could be more maintainable by:
- Creating a shared type for file-related items
- Using union types more effectively
+type FileType = 'file' | 'image' | 'audio'; +type FileItem = { + type: FileType; + file_id?: string; + file_url?: string; +}; export type ObjectStringItem = | { type: 'text'; text: string } - | { type: 'file'; file_id: string } - | { type: 'file'; file_url: string } - | { type: 'image'; file_id: string } - | { type: 'image'; file_url: string } - | { type: 'audio'; file_id: string } - | { type: 'audio'; file_url: string }; + | FileItem;Also applies to: 783-791
Line range hint
7-7: Replace Math.random-based UUID with a secure implementationThe current UUID implementation using
Math.random()and timestamp is not cryptographically secure and may produce collisions. This could lead to identification issues in a production environment.Consider using the
crypto.randomUUID()function or a well-tested UUID library:-const uuid = () => (Math.random() * new Date().getTime()).toString(); +const uuid = () => crypto.randomUUID();If you need to support older environments, consider using the
uuidpackage:import { v4 as uuidv4 } from 'uuid'; const uuid = () => uuidv4();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
examples/coze-js-web/src/pages/chat-x/index.tsx(1 hunks)packages/coze-js/src/resources/bots/bots.ts(3 hunks)packages/coze-js/src/resources/chat/chat.ts(8 hunks)packages/coze-js/src/resources/files/files.ts(1 hunks)packages/coze-js/src/resources/workflows/runs/runs.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/coze-js/src/resources/workflows/runs/runs.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/coze-js-web/src/pages/chat-x/index.tsx
🔇 Additional comments (2)
packages/coze-js/src/resources/files/files.ts (1)
49-64: Documentation improvements look good!
The translation of comments from Chinese to English is accurate and provides clear descriptions for the FileObject interface properties. This improves code accessibility for English-speaking developers.
packages/coze-js/src/resources/bots/bots.ts (1)
Line range hint 132-363: Documentation updates are well-structured and accurate!
The translation of comments from Chinese to English is thorough and maintains technical accuracy. The documentation now clearly describes:
- Bot configuration parameters
- Interface property purposes
- Type constraints and limitations
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## main #44 +/- ##
==========================================
+ Coverage 93.57% 94.19% +0.61%
==========================================
Files 68 40 -28
Lines 3549 2101 -1448
Branches 713 397 -316
==========================================
- Hits 3321 1979 -1342
+ Misses 227 122 -105
+ Partials 1 0 -1
|
Summary by CodeRabbit
Release Notes
New Features
ChatXcomponent for a chat interface, accessible via the new/chat-xroute.Settingscomponent for user-configurable application settings.useCozeAPIfor seamless interaction with the Coze API.useStylefor styling the chat interface.Enhancements
These updates enhance user experience with a new chat feature, customizable settings, and improved reliability.