Minor fixes 0731 2 (chat boosted) #1265
Conversation
📝 WalkthroughWalkthroughA chat feature with mention support was integrated into the desktop application's note editor area. This involved adding a chat button to the note header, replacing the chat input's textarea with a rich text editor supporting mentions, updating chat logic to handle mentioned notes and people, and propagating these changes through the chat view and logic layers. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NoteHeaderChips
participant RightPanel (Chat)
participant ChatInput
participant useChatLogic
User->>NoteHeaderChips: Clicks "Chat" button
NoteHeaderChips->>RightPanel (Chat): togglePanel("chat")
User->>ChatInput: Types message, adds mentions
ChatInput->>useChatLogic: onSubmit(message, mentionedContent)
useChatLogic->>useChatLogic: prepareMessageHistory(..., mentionedContent)
useChatLogic->>useChatLogic: Append mentioned notes/people content
useChatLogic-->>ChatInput: Message processed with context
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (1)
368-372: Remove legacy keyboard handler and rely on ChatInput’s onSubmitThe
handleKeyDowninuseChatLogicbypasses ChatInput’s mention-extraction and content-reset logic and is no longer needed—Editor already invokes theonSubmitprop when Enter is pressed.• In apps/desktop/src/components/right-panel/hooks/useChatLogic.ts
– Delete the entirehandleKeyDowndeclaration (lines 368–372)
– RemovehandleKeyDownfrom the returned object.• In apps/desktop/src/components/right-panel/views/chat-view.tsx
– Stop passinghandleKeyDownto<ChatInput … />(around line 167).• (Optional) Tidy up the ChatInputProps interface and Editor invocation in
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx by removing the now-unusedonKeyDownprop.Example diff in useChatLogic.ts:
- const handleKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => { - if (e.key === "Enter" && !e.shiftKey) { - e.preventDefault(); - handleSubmit(); - } - }; return { isGenerating, - handleKeyDown, handleSubmit, handleQuickAction, handleApplyMarkdown, };And in ChatView.tsx:
- <ChatInput - … - onSubmit={handleSubmit} - onKeyDown={handleKeyDown} - … - /> + <ChatInput + … + onSubmit={handleSubmit} + … + />
🧹 Nitpick comments (3)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (1)
137-212: Performance: Sequential await calls in loopMultiple
awaitcalls inside the loop can be slow. Consider parallelizing the data fetching.- for (const mention of mentionedContent) { + const mentionPromises = mentionedContent.map(async (mention) => { try { if (mention.type === "note") { const sessionData = await dbCommands.getSession({ id: mention.id }); if (sessionData) { // ... rest of note processing - noteContents.push(`\n\n--- Content from the note"${mention.label}" ---\n${noteContent}`); + return `\n\n--- Content from the note "${mention.label}" ---\n${noteContent}`; } } if (mention.type === "human") { // ... human processing - noteContents.push(`\n\n--- Content about the person "${mention.label}" ---\n${humanContent}`); + return `\n\n--- Content about the person "${mention.label}" ---\n${humanContent}`; } } catch (error) { console.error(`Error fetching content for "${mention.label}":`, error); + return null; } - } + }); + + const results = await Promise.all(mentionPromises); + const noteContents = results.filter(content => content !== null);apps/desktop/src/components/right-panel/views/chat-view.tsx (1)
170-170: Simplify arrow functionThe arrow function wrapper is unnecessary since you're just passing through the same arguments.
- onSubmit={(mentionedContent) => handleSubmit(mentionedContent)} + onSubmit={handleSubmit}apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
262-323: Move inline styles to external CSS fileThe large inline style block makes the component harder to maintain. Consider moving these styles to a separate CSS module.
Create a new file
chat-input.module.cssand import it:+import styles from './chat-input.module.css'; return ( <div className="border border-b-0 border-input mx-4 rounded-t-lg overflow-clip flex flex-col bg-white"> - {/* Custom styles to disable rich text features */} - <style> - {`...`} - </style>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/editor-area/note-header/chips/index.tsx(2 hunks)apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(5 hunks)apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(5 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/right-panel/views/chat-view.tsxapps/desktop/src/components/editor-area/note-header/chips/index.tsxapps/desktop/src/components/right-panel/hooks/useChatLogic.tsapps/desktop/src/components/right-panel/components/chat/chat-input.tsx
🔇 Additional comments (5)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (1)
187-194: DOM manipulation in potentially non-browser environmentUsing
document.createElementassumes a browser environment. This could fail in server-side rendering or testing environments.Consider using a more robust HTML parsing solution or adding environment checks:
- const div = document.createElement("div"); - div.innerHTML = session.enhanced_memo_html; - briefContent = (div.textContent || div.innerText || "").slice(0, 200) + "..."; + // Use a HTML parser library or check for browser environment + if (typeof document !== 'undefined') { + const div = document.createElement("div"); + div.innerHTML = session.enhanced_memo_html; + briefContent = (div.textContent || div.innerText || "").slice(0, 200) + "..."; + } else { + // Fallback for non-browser environments + briefContent = session.enhanced_memo_html.replace(/<[^>]*>/g, '').slice(0, 200) + "..."; + }apps/desktop/src/components/editor-area/note-header/chips/index.tsx (1)
8-24: Well-implemented chat button componentThe
StartChatButtoncomponent is cleanly implemented with proper event handling and styling.apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (3)
119-124: DOM manipulation without environment checkThe
extractPlainTextfunction uses DOM APIs that may not be available in all environments.const extractPlainText = useCallback((html: string) => { + if (typeof document === 'undefined') { + // Fallback for non-browser environments + return html.replace(/<[^>]*>/g, ''); + } const div = document.createElement("div"); div.innerHTML = html; return div.textContent || div.innerText || ""; }, []);
138-181: Add error handling to mention extractionThe
extractMentionedContentfunction should handle potential errors during JSON traversal.const extractMentionedContent = useCallback(() => { + try { if (!editorRef.current?.editor) { return []; } const doc = editorRef.current.editor.getJSON(); // ... rest of the function return mentions; + } catch (error) { + console.error('Failed to extract mentions:', error); + return []; + } }, []);Likely an incorrect or invalid review comment.
79-117: Add error handling for database operationsThe
handleMentionSearchfunction lacks error handling for database calls, which could cause the mention search to fail silently.const handleMentionSearch = useCallback(async (query: string) => { + try { const now = Date.now(); const timeSinceLastEvent = now - lastBacklinkSearchTime.current; // ... existing code ... return [...noteResults, ...peopleResults].slice(0, 5); + } catch (error) { + console.error('Failed to search mentions:', error); + return []; + } }, [userId]);Likely an incorrect or invalid review comment.
| editor.view.dom.removeEventListener("click", handleClick); | ||
| }; | ||
| } | ||
| }, [editorRef.current?.editor, onKeyDown, handleSubmit, inputValue]); |
There was a problem hiding this comment.
Unstable dependency in useEffect
Including editorRef.current?.editor in the dependency array can cause the effect to run on every render since the editor instance might change.
- }, [editorRef.current?.editor, onKeyDown, handleSubmit, inputValue]);
+ }, [onKeyDown, handleSubmit, inputValue]);Instead, check for editor existence inside the effect and use a ref to track if listeners are already attached.
📝 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.
| }, [editorRef.current?.editor, onKeyDown, handleSubmit, inputValue]); | |
| }, [onKeyDown, handleSubmit, inputValue]); |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/components/chat/chat-input.tsx at
line 243, remove editorRef.current?.editor from the useEffect dependency array
to prevent the effect from running on every render due to unstable reference
changes. Instead, inside the effect, check if the editor instance exists and use
a separate ref to track whether event listeners have already been attached,
ensuring listeners are added only once.
| if (mentionedContent && mentionedContent.length > 0) { | ||
| currentUserMessage += | ||
| "[[From here is an automatically appended content from the mentioned notes & people, not what the user wrote. Use this only as a reference for more context. Your focus should always be the current meeting user is viewing]]" | ||
| + "\n\n"; | ||
| } | ||
|
|
||
| if (mentionedContent && mentionedContent.length > 0) { |
There was a problem hiding this comment.
Duplicate condition check for mentionedContent
The same condition if (mentionedContent && mentionedContent.length > 0) is checked twice. Consider combining these blocks.
- if (mentionedContent && mentionedContent.length > 0) {
- currentUserMessage +=
- "[[From here is an automatically appended content from the mentioned notes & people, not what the user wrote. Use this only as a reference for more context. Your focus should always be the current meeting user is viewing]]"
- + "\n\n";
- }
-
- if (mentionedContent && mentionedContent.length > 0) {
+ if (mentionedContent && mentionedContent.length > 0) {
+ currentUserMessage +=
+ "[[From here is an automatically appended content from the mentioned notes & people, not what the user wrote. Use this only as a reference for more context. Your focus should always be the current meeting user is viewing]]"
+ + "\n\n";
+
const noteContents: string[] = [];📝 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.
| if (mentionedContent && mentionedContent.length > 0) { | |
| currentUserMessage += | |
| "[[From here is an automatically appended content from the mentioned notes & people, not what the user wrote. Use this only as a reference for more context. Your focus should always be the current meeting user is viewing]]" | |
| + "\n\n"; | |
| } | |
| if (mentionedContent && mentionedContent.length > 0) { | |
| if (mentionedContent && mentionedContent.length > 0) { | |
| currentUserMessage += | |
| "[[From here is an automatically appended content from the mentioned notes & people, not what the user wrote. Use this only as a reference for more context. Your focus should always be the current meeting user is viewing]]" | |
| + "\n\n"; | |
| const noteContents: string[] = []; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/hooks/useChatLogic.ts around lines
128 to 134, the condition checking if mentionedContent exists and has length
greater than zero is duplicated. Combine these two if blocks into one to avoid
redundant checks by merging their contents under a single if statement that
checks mentionedContent and its length once.
| continue; | ||
| } | ||
|
|
||
| noteContents.push(`\n\n--- Content from the note"${mention.label}" ---\n${noteContent}`); |
There was a problem hiding this comment.
Missing space in string concatenation
Missing space between "note" and the quote character in the string template.
- noteContents.push(`\n\n--- Content from the note"${mention.label}" ---\n${noteContent}`);
+ noteContents.push(`\n\n--- Content from the note "${mention.label}" ---\n${noteContent}`);📝 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.
| noteContents.push(`\n\n--- Content from the note"${mention.label}" ---\n${noteContent}`); | |
| noteContents.push(`\n\n--- Content from the note "${mention.label}" ---\n${noteContent}`); |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/hooks/useChatLogic.ts at line 153,
there is a missing space between the word "note" and the quote character in the
string template. Fix this by adding a space after "note" so the string reads
`note "${mention.label}"` instead of `note"${mention.label}"`.
| if (messages.length >= 14 && !getLicense.data?.valid) { | ||
| if (userId) { | ||
| await analyticsCommands.event({ | ||
| event: "pro_license_required_chat", | ||
| distinct_id: userId, | ||
| }); | ||
| } | ||
| await message("2 messages are allowed per conversation for free users.", { | ||
| await message("7 messages are allowed per conversation for free users.", { |
There was a problem hiding this comment.
Inconsistent message limit logic
The code checks for 14 messages but displays "7 messages" to the user. This seems inconsistent - if you're counting individual messages (user + assistant), then 14 messages would be 7 conversation turns.
Either update the check or the message to be consistent:
- if (messages.length >= 14 && !getLicense.data?.valid) {
+ if (messages.length >= 14 && !getLicense.data?.valid) {
// ...
- await message("7 messages are allowed per conversation for free users.", {
+ await message("7 conversation turns (14 messages) are allowed per conversation for free users.", {🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/hooks/useChatLogic.ts around lines
238 to 245, the code checks if messages.length is at least 14 but the user
message states "7 messages are allowed," causing inconsistency. Fix this by
either changing the condition to check for 7 messages or updating the displayed
message to reflect 14 messages, ensuring the limit logic and user notification
match.
1 participant
No description provided.