Conversation
📝 WalkthroughWalkthroughIntroduces a new useChatInput hook and rewires the desktop ChatView to use it. Extracts EmptyChatState UI into the UI package. Refactors chat input, message rendering, and markdown handling across UI components. Adds types/utilities, updates ChatModelInfoModal API, and propagates sessionTitle in selection data. Updates Tailwind content and UI package dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ChatInput
participant useChatInput
participant Editor
participant ModelModal
rect rgb(245,248,255)
note over useChatInput: Initialize queries and state
useChatInput->>useChatInput: Fetch LLM connection / model / HyprCloud
useChatInput->>useChatInput: Fetch entity data (note/human/org)
end
User->>ChatInput: Type / Mention / Submit
ChatInput->>useChatInput: handleMentionSearch(query, editor)
useChatInput-->>ChatInput: results
ChatInput->>useChatInput: processSelection(pendingSelection)
useChatInput-->>ChatInput: {id, html, text}
ChatInput->>useChatInput: handleSubmit(mentionedContent, selectionData, htmlContent)
useChatInput->>useChatInput: clearPendingSelection()
User->>ModelModal: Choose model
ModelModal->>ChatInput: onChooseModel()
ChatInput-->>ChatView: onChooseModel()
note over ChatView: Open settings and navigate to AI-LLM
sequenceDiagram
autonumber
participant MessagesView
participant UIMessage
participant MarkdownParser
participant MarkdownCard
participant Converter
MessagesView->>UIMessage: Render message parts
alt text part
UIMessage->>MarkdownParser: parseMarkdownBlocks(text)
MarkdownParser-->>UIMessage: [parts]
loop each parsed part
alt markdown block
UIMessage->>MarkdownCard: render(content, isComplete, convertMarkdownToHtml?)
opt converter provided
MarkdownCard->>Converter: convertMarkdownToHtml(markdown)
Converter-->>MarkdownCard: html
end
else plain text
UIMessage-->>MessagesView: render TextContent (with optional converter)
end
end
else non-text part
UIMessage-->>MessagesView: render accordingly
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,ts,tsx,rs}⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (4)packages/ui/src/components/chat/chat-input.tsx (1)
apps/desktop/src/components/right-panel/chat-view.tsx (2)
apps/desktop/src/components/editor-area/text-selection-popover.tsx (1)
apps/desktop/src/components/right-panel/hooks/useChatInput.ts (3)
🪛 ast-grep (0.39.5)apps/desktop/src/components/right-panel/hooks/useChatInput.ts[warning] 151-151: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first. (dom-content-modification) [warning] 151-151: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify. (unsafe-html-content-assignment) ⏰ 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)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
205-223: Restore parent onKeyDown propagationThe refactor now captures editor keydown events but never forwards them to the
onKeyDowncallback, so any parent logic (shortcuts, analytics, etc.) tied to that prop no longer runs. Please propagate the event after the internal handling.const handleKeyDown = (event: KeyboardEvent) => { if (event.metaKey || event.ctrlKey) { if (["b", "i", "u", "k"].includes(event.key.toLowerCase())) { event.preventDefault(); return; } } if (event.key === "Enter" && !event.shiftKey) { event.preventDefault(); if (inputValue.trim()) { handleSubmit(); } } + + onKeyDown?.(event as unknown as React.KeyboardEvent<HTMLTextAreaElement>); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(6 hunks)apps/desktop/src/components/right-panel/components/chat/ui-message.tsx(2 hunks)apps/desktop/src/components/right-panel/hooks/useChatInput.ts(1 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(1 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/chat/index.ts(1 hunks)packages/ui/src/components/chat/ui-message.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
packages/ui/src/components/chat/index.tspackages/ui/src/components/chat/ui-message.tsxapps/desktop/src/components/right-panel/hooks/useChatInput.tsapps/desktop/src/components/right-panel/views/chat-view.tsxapps/desktop/src/components/right-panel/components/chat/chat-input.tsxapps/desktop/src/components/right-panel/components/chat/ui-message.tsx
🧬 Code graph analysis (4)
packages/ui/src/components/chat/ui-message.tsx (3)
apps/desktop/src/components/right-panel/utils/markdown-parser.ts (2)
ParsedPart(2-6)parseMarkdownBlocks(8-67)apps/desktop/src/components/right-panel/components/chat/ui-message.tsx (1)
UIMessageComponent(15-30)apps/desktop/src/components/right-panel/components/chat/markdown-card.tsx (1)
MarkdownCard(15-140)
apps/desktop/src/components/right-panel/hooks/useChatInput.ts (3)
apps/desktop/src/contexts/hypr.tsx (1)
useHypr(63-69)apps/desktop/src/contexts/right-panel.tsx (1)
useRightPanel(230-236)packages/db/src/schema.ts (2)
sessions(27-34)humans(13-18)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (2)
apps/desktop/src/contexts/right-panel.tsx (1)
useRightPanel(230-236)apps/desktop/src/components/right-panel/hooks/useChatInput.ts (1)
useChatInput(18-194)
apps/desktop/src/components/right-panel/components/chat/ui-message.tsx (3)
packages/ui/src/components/chat/ui-message.tsx (1)
UIMessageComponent(177-509)apps/desktop/src/components/right-panel/utils/markdown-parser.ts (1)
parseMarkdownBlocks(8-67)apps/desktop/src/components/right-panel/components/chat/markdown-card.tsx (1)
MarkdownCard(15-140)
🪛 ast-grep (0.39.5)
apps/desktop/src/components/right-panel/hooks/useChatInput.ts
[warning] 149-149: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: temp.innerHTML = selectedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 149-149: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: temp.innerHTML = selectedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
⏰ 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)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
| let html = await convertMarkdownToHtml(content); | ||
|
|
||
| // Clean up empty paragraphs | ||
| html = html | ||
| .replace(/<p>\s*<\/p>/g, "") | ||
| .replace(/<p>\u00A0<\/p>/g, "") | ||
| .replace(/<p> <\/p>/g, "") | ||
| .replace(/<p>\s+<\/p>/g, "") | ||
| .replace(/<p> <\/p>/g, "") | ||
| .trim(); | ||
|
|
||
| setDisplayHtml(html); | ||
| } catch (error) { | ||
| console.error("Failed to convert markdown:", error); | ||
| setDisplayHtml(content); | ||
| } | ||
| } else { | ||
| // Fallback: display as plain text in a paragraph | ||
| setDisplayHtml(`<p>${content}</p>`); | ||
| } |
There was a problem hiding this comment.
Sanitize TextContent fallbacks to avoid XSS.
When convertMarkdownToHtml throws or isn't supplied we drop raw content into Renderer, which will render unsanitized HTML and allows script injection. Escape the fallback path before handing it to Renderer.
+const ESCAPE_HTML_MAP: Record<string, string> = {
+ "&": "&",
+ "<": "<",
+ ">": ">",
+ '"': """,
+ "'": "'",
+};
+
+const escapeHtml = (value: string) =>
+ value.replace(/[&<>"']/g, (char) => ESCAPE_HTML_MAP[char] ?? char);
+
const TextContent: FC<{
content: string;
isHtml?: boolean;
convertMarkdownToHtml?: (markdown: string) => Promise<string>;
}> = ({ content, isHtml, convertMarkdownToHtml }) => {
const [displayHtml, setDisplayHtml] = useState<string>("");
@@
if (convertMarkdownToHtml) {
try {
let html = await convertMarkdownToHtml(content);
@@
setDisplayHtml(html);
} catch (error) {
console.error("Failed to convert markdown:", error);
- setDisplayHtml(content);
+ setDisplayHtml(`<p>${escapeHtml(content)}</p>`);
}
} else {
// Fallback: display as plain text in a paragraph
- setDisplayHtml(`<p>${content}</p>`);
+ setDisplayHtml(`<p>${escapeHtml(content)}</p>`);
}
};📝 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.
| let html = await convertMarkdownToHtml(content); | |
| // Clean up empty paragraphs | |
| html = html | |
| .replace(/<p>\s*<\/p>/g, "") | |
| .replace(/<p>\u00A0<\/p>/g, "") | |
| .replace(/<p> <\/p>/g, "") | |
| .replace(/<p>\s+<\/p>/g, "") | |
| .replace(/<p> <\/p>/g, "") | |
| .trim(); | |
| setDisplayHtml(html); | |
| } catch (error) { | |
| console.error("Failed to convert markdown:", error); | |
| setDisplayHtml(content); | |
| } | |
| } else { | |
| // Fallback: display as plain text in a paragraph | |
| setDisplayHtml(`<p>${content}</p>`); | |
| } | |
| const ESCAPE_HTML_MAP: Record<string, string> = { | |
| "&": "&", | |
| "<": "<", | |
| ">": ">", | |
| '"': """, | |
| "'": "'", | |
| }; | |
| const escapeHtml = (value: string) => | |
| value.replace(/[&<>"']/g, (char) => ESCAPE_HTML_MAP[char] ?? char); | |
| const TextContent: FC<{ | |
| content: string; | |
| isHtml?: boolean; | |
| convertMarkdownToHtml?: (markdown: string) => Promise<string>; | |
| }> = ({ content, isHtml, convertMarkdownToHtml }) => { | |
| const [displayHtml, setDisplayHtml] = useState<string>(""); | |
| useEffect(() => { | |
| const renderContent = async () => { | |
| if (convertMarkdownToHtml) { | |
| try { | |
| let html = await convertMarkdownToHtml(content); | |
| // Clean up empty paragraphs | |
| html = html | |
| .replace(/<p>\s*<\/p>/g, "") | |
| .replace(/<p>\u00A0<\/p>/g, "") | |
| .replace(/<p> <\/p>/g, "") | |
| .replace(/<p>\s+<\/p>/g, "") | |
| .replace(/<p> <\/p>/g, "") | |
| .trim(); | |
| setDisplayHtml(html); | |
| } catch (error) { | |
| console.error("Failed to convert markdown:", error); | |
| setDisplayHtml(`<p>${escapeHtml(content)}</p>`); | |
| } | |
| } else { | |
| // Fallback: display as plain text in a paragraph | |
| setDisplayHtml(`<p>${escapeHtml(content)}</p>`); | |
| } | |
| }; | |
| renderContent(); | |
| }, [content, convertMarkdownToHtml]); | |
| return <Renderer html={displayHtml} />; | |
| }; |
🤖 Prompt for AI Agents
In packages/ui/src/components/chat/ui-message.tsx around lines 49 to 68, the
fallback paths setDisplayHtml(content) and setDisplayHtml(`<p>${content}</p>`)
inject raw user content into the renderer and allow XSS; sanitize or escape the
content before calling setDisplayHtml. Replace those fallbacks to either run the
content through the same HTML sanitizer used elsewhere (e.g.,
DOMPurify.sanitize) or escape HTML entities (convert <, >, &, ", ' to entities)
and then wrap in a safe paragraph, and update imports to use the project's
sanitizer utility or add a small escapeHtml helper; ensure you sanitize both the
error catch fallback and the plain-text fallback so no raw HTML is ever passed
to setDisplayHtml.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/components/right-panel/components/chat/empty-chat-state.tsx (1)
28-37: TS compile issue: React.MouseEvent type used without import; also unused param.The returned handler types
React.MouseEventbutReactisn’t imported as a type. Since the event isn’t used, drop it to avoid the import and keep types tight.- const handleButtonClick = useCallback((prompt: string, analyticsEvent: string) => (e: React.MouseEvent) => { + const handleButtonClick = useCallback((prompt: string, analyticsEvent: string) => () => { if (userId) { analyticsCommands.event({ event: analyticsEvent, distinct_id: userId, }); } onQuickAction(prompt); }, [onQuickAction, userId]);apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
204-241: Duplicate Enter handling; possible double submit and dropdown bypass.You handle Enter in both TipTap’s handleKeyDown (Lines 174-202) and a DOM keydown listener here. The DOM listener:
- May double-trigger submit.
- Ignores the mention dropdown check present in the TipTap handler.
Remove this effect and rely on the TipTap handler.
- useEffect(() => { - const editor = editorRef.current?.editor; - if (editor) { - const handleKeyDown = (event: KeyboardEvent) => { - if (event.metaKey || event.ctrlKey) { - if (["b", "i", "u", "k"].includes(event.key.toLowerCase())) { - event.preventDefault(); - return; - } - } - - if (event.key === "Enter" && !event.shiftKey) { - event.preventDefault(); - - if (inputValue.trim()) { - handleSubmit(); - } - } - }; - - const handleClick = (event: MouseEvent) => { - const target = event.target as HTMLElement; - if (target && (target.classList.contains("mention") || target.closest(".mention"))) { - event.preventDefault(); - event.stopPropagation(); - return false; - } - }; - - editor.view.dom.addEventListener("keydown", handleKeyDown); - editor.view.dom.addEventListener("click", handleClick); - - return () => { - editor.view.dom.removeEventListener("keydown", handleKeyDown); - editor.view.dom.removeEventListener("click", handleClick); - }; - } - }, [editorRef.current?.editor, onKeyDown, handleSubmit, inputValue]); + // (Removed duplicate DOM keydown/click listeners; TipTap handleKeyDown covers Enter + dropdown cases.)
🧹 Nitpick comments (4)
apps/desktop/src/components/right-panel/components/chat/index.ts (1)
6-6: Redundant re-export; prefer a single pattern.Line 6 re-exports EmptyChatState that is already covered by export * from "./empty-chat-state" on Line 5. Keep one to avoid duplication noise.
Apply:
-export { EmptyChatState } from "./empty-chat-state";packages/ui/src/components/chat/empty-chat-state.tsx (2)
19-21: Don’t destructure unused props to satisfy “no unused vars”.onQuickAction, onFocusInput, sessionId, userId aren’t used in this UI-only component.
- { onQuickAction, onFocusInput, sessionId, userId, handleButtonClick, handleContainerClick, quickActions }: + { handleButtonClick, handleContainerClick, quickActions }: EmptyChatStateProps,
110-118: Prefer stable keys over array index (optional).If possible, derive a stable key (e.g., eventName or a composite) to avoid re-mounts on reorders.
- {quickActions.map((action, index) => ( + {quickActions.map((action) => ( <button - key={index} + key={`${action.eventName}:${action.actualPrompt}`} onClick={handleButtonClick(action.actualPrompt, action.eventName)}apps/desktop/src/components/right-panel/hooks/useChatInput.ts (1)
25-45: Polling frequency consideration (optional).Multiple queries poll every 5s. Consider adding staleTime and/or increasing intervals to reduce load.
Example:
- const llmConnectionQuery = useQuery({ ..., refetchInterval: 5000 }); + const llmConnectionQuery = useQuery({ ..., staleTime: 10_000, refetchInterval: 15_000 });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/desktop/src/locales/en/messages.pois excluded by!**/*.poapps/desktop/src/locales/ko/messages.pois excluded by!**/*.popnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(6 hunks)apps/desktop/src/components/right-panel/components/chat/empty-chat-state.tsx(4 hunks)apps/desktop/src/components/right-panel/components/chat/index.ts(1 hunks)apps/desktop/src/components/right-panel/hooks/useChatInput.ts(1 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(1 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/chat/empty-chat-state.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/components/right-panel/views/chat-view.tsx
- packages/ui/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
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/components/chat/index.tsapps/desktop/src/components/right-panel/components/chat/chat-input.tsxapps/desktop/src/components/right-panel/hooks/useChatInput.tspackages/ui/src/components/chat/empty-chat-state.tsxapps/desktop/src/components/right-panel/components/chat/empty-chat-state.tsx
🧬 Code graph analysis (3)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (2)
packages/utils/src/contexts/right-panel.tsx (1)
useRightPanel(112-118)apps/desktop/src/components/right-panel/hooks/useChatInput.ts (1)
useChatInput(19-195)
apps/desktop/src/components/right-panel/hooks/useChatInput.ts (3)
apps/desktop/src/contexts/hypr.tsx (1)
useHypr(63-69)packages/utils/src/contexts/right-panel.tsx (1)
useRightPanel(112-118)packages/db/src/schema.ts (2)
sessions(34-42)humans(13-18)
apps/desktop/src/components/right-panel/components/chat/empty-chat-state.tsx (2)
apps/desktop/src/components/right-panel/utils/dynamic-quickactions.ts (1)
getDynamicQuickActions(33-95)packages/ui/src/components/chat/empty-chat-state.tsx (1)
EmptyChatStateUI(17-123)
🪛 ast-grep (0.39.5)
apps/desktop/src/components/right-panel/hooks/useChatInput.ts
[warning] 150-150: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: temp.innerHTML = selectedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 150-150: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: temp.innerHTML = selectedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
⏰ 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)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (2)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
174-202: Do we still need the onKeyDown prop?onKeyDown is no longer invoked. Confirm if deprecation is intended. If not, call it inside the TipTap handleKeyDown path before custom logic.
Option:
handleKeyDown: (view, event) => { + // Bubble to external handler if provided + if (typeof onKeyDown === "function") { + // Cast to satisfy TS; external consumers expect React event-like shape. + onKeyDown(event as unknown as React.KeyboardEvent<HTMLTextAreaElement>); + }apps/desktop/src/components/right-panel/hooks/useChatInput.ts (1)
73-77: full_name property validated plugin-db’s Human type includesfull_name(string | null) and DB schema/migrations/seeds all usefull_name, sohumanData?.full_nameis correct.
| const processSelection = useCallback((selection: any) => { | ||
| if (!selection) { | ||
| return null; | ||
| } | ||
|
|
||
| const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note"; | ||
| const selectedHtml = selection.text || ""; | ||
|
|
||
| // Strip HTML tags | ||
| const temp = document.createElement("div"); | ||
| temp.innerHTML = selectedHtml; | ||
| const selectedText = (temp.textContent || temp.innerText || "").trim(); | ||
|
|
||
| const textPreview = selectedText.length > 0 | ||
| ? (selectedText.length > 6 | ||
| ? `'${selectedText.slice(0, 6)}...'` | ||
| : `'${selectedText}'`) | ||
| : "NO_TEXT"; | ||
|
|
||
| const selectionRef = textPreview !== "NO_TEXT" | ||
| ? `[${noteName} - ${textPreview}(${selection.startOffset}:${selection.endOffset})]` | ||
| : `[${noteName} - ${selection.startOffset}:${selection.endOffset}]`; | ||
|
|
||
| const escapedSelectionRef = selectionRef.replace(/"/g, """); | ||
|
|
||
| return { | ||
| html: | ||
| `<a class="mention selection-ref" data-mention="true" data-id="selection-${selection.startOffset}-${selection.endOffset}" data-type="selection" data-label="${escapedSelectionRef}" contenteditable="false">${selectionRef}</a> `, | ||
| text: selectionRef, | ||
| id: `${selection.startOffset}-${selection.endOffset}-${selection.timestamp}`, | ||
| }; | ||
| }, [noteData?.title, humanData?.full_name, organizationData?.name]); |
There was a problem hiding this comment.
XSS/sanitization: building HTML with unescaped text and innerHTML usage.
- Using innerHTML to parse user-provided HTML is flagged; also, selectionRef is injected as inner HTML without escaping, allowing malformed markup injection.
Apply safer parsing and escaping for both attribute and inner text:
- const processSelection = useCallback((selection: any) => {
+ const processSelection = useCallback((selection: any) => {
if (!selection) {
return null;
}
const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note";
const selectedHtml = selection.text || "";
- // Strip HTML tags
- const temp = document.createElement("div");
- temp.innerHTML = selectedHtml;
- const selectedText = (temp.textContent || temp.innerText || "").trim();
+ // Extract plain text safely (avoid innerHTML on unsanitized content)
+ const parser = new DOMParser();
+ const doc = parser.parseFromString(selectedHtml, "text/html");
+ const selectedText = (doc.body.textContent || "").trim();
const textPreview = selectedText.length > 0
? (selectedText.length > 6
? `'${selectedText.slice(0, 6)}...'`
: `'${selectedText}'`)
: "NO_TEXT";
const selectionRef = textPreview !== "NO_TEXT"
? `[${noteName} - ${textPreview}(${selection.startOffset}:${selection.endOffset})]`
: `[${noteName} - ${selection.startOffset}:${selection.endOffset}]`;
- const escapedSelectionRef = selectionRef.replace(/"/g, """);
+ const escapeAttr = (s: string) =>
+ s.replace(/&/g, "&").replace(/"/g, """).replace(/</g, "<").replace(/>/g, ">");
+ const escapeText = (s: string) =>
+ s.replace(/&/g, "&").replace(/</g, "<").replace(/>/g, ">");
+ const escapedLabel = escapeAttr(selectionRef);
+ const escapedInner = escapeText(selectionRef);
return {
- html:
- `<a class="mention selection-ref" data-mention="true" data-id="selection-${selection.startOffset}-${selection.endOffset}" data-type="selection" data-label="${escapedSelectionRef}" contenteditable="false">${selectionRef}</a> `,
+ html:
+ `<a class="mention selection-ref" data-mention="true" data-id="selection-${selection.startOffset}-${selection.endOffset}" data-type="selection" data-label="${escapedLabel}" contenteditable="false">${escapedInner}</a> `,
text: selectionRef,
id: `${selection.startOffset}-${selection.endOffset}-${selection.timestamp}`,
};
- }, [noteData?.title, humanData?.full_name, organizationData?.name]);
+ }, [noteData?.title, humanData?.full_name, organizationData?.name]);Based on static analysis hints
📝 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 processSelection = useCallback((selection: any) => { | |
| if (!selection) { | |
| return null; | |
| } | |
| const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note"; | |
| const selectedHtml = selection.text || ""; | |
| // Strip HTML tags | |
| const temp = document.createElement("div"); | |
| temp.innerHTML = selectedHtml; | |
| const selectedText = (temp.textContent || temp.innerText || "").trim(); | |
| const textPreview = selectedText.length > 0 | |
| ? (selectedText.length > 6 | |
| ? `'${selectedText.slice(0, 6)}...'` | |
| : `'${selectedText}'`) | |
| : "NO_TEXT"; | |
| const selectionRef = textPreview !== "NO_TEXT" | |
| ? `[${noteName} - ${textPreview}(${selection.startOffset}:${selection.endOffset})]` | |
| : `[${noteName} - ${selection.startOffset}:${selection.endOffset}]`; | |
| const escapedSelectionRef = selectionRef.replace(/"/g, """); | |
| return { | |
| html: | |
| `<a class="mention selection-ref" data-mention="true" data-id="selection-${selection.startOffset}-${selection.endOffset}" data-type="selection" data-label="${escapedSelectionRef}" contenteditable="false">${selectionRef}</a> `, | |
| text: selectionRef, | |
| id: `${selection.startOffset}-${selection.endOffset}-${selection.timestamp}`, | |
| }; | |
| }, [noteData?.title, humanData?.full_name, organizationData?.name]); | |
| const processSelection = useCallback((selection: any) => { | |
| if (!selection) { | |
| return null; | |
| } | |
| const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note"; | |
| const selectedHtml = selection.text || ""; | |
| // Extract plain text safely (avoid innerHTML on unsanitized content) | |
| const parser = new DOMParser(); | |
| const doc = parser.parseFromString(selectedHtml, "text/html"); | |
| const selectedText = (doc.body.textContent || "").trim(); | |
| const textPreview = selectedText.length > 0 | |
| ? (selectedText.length > 6 | |
| ? `'${selectedText.slice(0, 6)}...'` | |
| : `'${selectedText}'`) | |
| : "NO_TEXT"; | |
| const selectionRef = textPreview !== "NO_TEXT" | |
| ? `[${noteName} - ${textPreview}(${selection.startOffset}:${selection.endOffset})]` | |
| : `[${noteName} - ${selection.startOffset}:${selection.endOffset}]`; | |
| const escapeAttr = (s: string) => | |
| s.replace(/&/g, "&") | |
| .replace(/"/g, """) | |
| .replace(/</g, "<") | |
| .replace(/>/g, ">"); | |
| const escapeText = (s: string) => | |
| s.replace(/&/g, "&") | |
| .replace(/</g, "<") | |
| .replace(/>/g, ">"); | |
| const escapedLabel = escapeAttr(selectionRef); | |
| const escapedInner = escapeText(selectionRef); | |
| return { | |
| html: | |
| `<a class="mention selection-ref"` + | |
| ` data-mention="true"` + | |
| ` data-id="selection-${selection.startOffset}-${selection.endOffset}"` + | |
| ` data-type="selection"` + | |
| ` data-label="${escapedLabel}"` + | |
| ` contenteditable="false">${escapedInner}</a> `, | |
| text: selectionRef, | |
| id: `${selection.startOffset}-${selection.endOffset}-${selection.timestamp}`, | |
| }; | |
| }, [noteData?.title, humanData?.full_name, organizationData?.name]); |
🧰 Tools
🪛 ast-grep (0.39.5)
[warning] 150-150: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: temp.innerHTML = selectedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 150-150: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: temp.innerHTML = selectedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
| interface EmptyChatStateProps { | ||
| onQuickAction: (prompt: string) => void; | ||
| onFocusInput: () => void; | ||
| sessionId?: string | null; | ||
| userId?: string | null; | ||
| handleButtonClick: (prompt: string, analyticsEvent: string) => (e: React.MouseEvent) => void; | ||
| handleContainerClick: () => void; | ||
| quickActions: Array<{ | ||
| shownTitle: string; | ||
| actualPrompt: string; | ||
| eventName: string; | ||
| }>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid React.MouseEvent type dependency and keep prop surface minimal.
- Remove React.MouseEvent reference to prevent missing type import issues.
- Keep props interface unchanged for compatibility, but type the click factory to return () => void.
interface EmptyChatStateProps {
onQuickAction: (prompt: string) => void;
onFocusInput: () => void;
sessionId?: string | null;
userId?: string | null;
- handleButtonClick: (prompt: string, analyticsEvent: string) => (e: React.MouseEvent) => void;
+ handleButtonClick: (prompt: string, analyticsEvent: string) => () => void;
handleContainerClick: () => void;
quickActions: Array<{
shownTitle: string;
actualPrompt: string;
eventName: 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.
| interface EmptyChatStateProps { | |
| onQuickAction: (prompt: string) => void; | |
| onFocusInput: () => void; | |
| sessionId?: string | null; | |
| userId?: string | null; | |
| handleButtonClick: (prompt: string, analyticsEvent: string) => (e: React.MouseEvent) => void; | |
| handleContainerClick: () => void; | |
| quickActions: Array<{ | |
| shownTitle: string; | |
| actualPrompt: string; | |
| eventName: string; | |
| }>; | |
| } | |
| interface EmptyChatStateProps { | |
| onQuickAction: (prompt: string) => void; | |
| onFocusInput: () => void; | |
| sessionId?: string | null; | |
| userId?: string | null; | |
| handleButtonClick: (prompt: string, analyticsEvent: string) => () => void; | |
| handleContainerClick: () => void; | |
| quickActions: Array<{ | |
| shownTitle: string; | |
| actualPrompt: string; | |
| eventName: string; | |
| }>; | |
| } |
🤖 Prompt for AI Agents
In packages/ui/src/components/chat/empty-chat-state.tsx around lines 3 to 15,
the handleButtonClick prop currently types its return as (e: React.MouseEvent)
=> void which forces a React type import and expands the prop surface; change
the signature to (prompt: string, analyticsEvent: string) => () => void to
remove the React.MouseEvent dependency and avoid needing to import React types,
and update any local implementations/call sites to accept/return a no-arg
function (or ignore the event parameter) so behavior stays the same while the
prop type is simplified.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src/components/right-panel/components/chat-model-info-modal.tsx (1)
4-23: Wire up the newonChooseModelprop.
ChatModelInfoModalPropsnow requiresonChooseModel, but the component never invokes it. Any consumer passing analytics/telemetry (or alternative navigation) logic will be ignored, so the new API surface is effectively broken.Call the provided callback inside
handleChooseModelbefore (or after) the built-in settings navigation so external code can piggyback on the event without losing the existing behavior.-export function ChatModelInfoModal({ isOpen, onClose }: ChatModelInfoModalProps) { - const handleChooseModel = () => { +export function ChatModelInfoModal({ isOpen, onClose, onChooseModel }: ChatModelInfoModalProps) { + const handleChooseModel = () => { + onChooseModel?.(); windowsCommands.windowShow({ type: "settings" }).then(() => { setTimeout(() => { windowsCommands.windowEmitNavigate({ type: "settings" }, { path: "/app/settings", search: { tab: "ai-llm" }, }); }, 800); }); onClose(); };apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (2)
174-202: Remove ref from dependency array.
editorRef.current?.editorshould not be in the dependency array because refs are not reactive. Changes toref.currentdo not trigger re-renders, so the effect won't re-run when the editor instance changes. This can lead to stale closures.Apply this diff:
} - }, [editorRef.current?.editor, inputValue, handleSubmit]); + }, [inputValue, handleSubmit]);If you need to react to editor initialization, consider using a state variable instead of a ref, or move the setup logic to where the editor is created.
204-241: Remove ref from dependency array.Same issue as above:
editorRef.current?.editoris not reactive and should not be in the dependency array.Apply this diff:
}; } - }, [editorRef.current?.editor, onKeyDown, handleSubmit, inputValue]); + }, [onKeyDown, handleSubmit, inputValue]);
🧹 Nitpick comments (5)
apps/desktop/src/components/right-panel/views/chat-view.tsx (1)
19-24: Use JSX to renderChatViewInner.We’re invoking
ChatViewInner()as a plain function. React technically tolerates it, but you lose a real component boundary (DevTools tree readability, error stacks, potential hook linting/glitch scenarios). Prefer<ChatViewInner />(or inline the body) so React continues to treat it as a component.packages/ui/src/components/chat/chat-messages-view.tsx (1)
49-112: AvoidNodeJS.Timeoutin browser UI code.
NodeJS.Timeoutties this component to Node’s ambient types. If the UI package ever drops@types/node, this will stop compiling. Stick withReturnType<typeof setTimeout>to keep it environment-agnostic.- const thinkingTimeoutRef = useRef<NodeJS.Timeout | null>(null); + const thinkingTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);packages/ui/src/components/chat/types.ts (1)
14-14: Preferunknownoveranyfor toolDetails.Using
anybypasses type checking. Consider usingunknownor defining a specific type for toolDetails to maintain type safety.Apply this diff:
- toolDetails?: any; + toolDetails?: unknown;If toolDetails has a known structure, define a proper interface instead.
packages/ui/src/components/chat/markdown-card.tsx (2)
11-11: Remove unused parameter.The
hasEnhancedNoteparameter is defined but never used in the component. Consider removing it to reduce confusion.Apply this diff:
interface MarkdownCardProps { content: string; isComplete: boolean; sessionTitle?: string; onApplyMarkdown?: (markdownContent: string) => void; - hasEnhancedNote?: boolean; convertMarkdownToHtml?: (markdown: string) => Promise<string>; } export function MarkdownCard( - { content, isComplete, sessionTitle, onApplyMarkdown, hasEnhancedNote = false, convertMarkdownToHtml }: + { content, isComplete, sessionTitle, onApplyMarkdown, convertMarkdownToHtml }: MarkdownCardProps, ) {Also applies to: 16-16
55-111: Consider externalizing styles.The extensive inline styles with
!importantdeclarations suggest potential conflicts with existing CSS. Consider moving these styles to a separate CSS module or using a CSS-in-JS solution to improve maintainability.This is a minor maintainability concern. If the
!importantoverrides are necessary for TipTap integration, document why they're required.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
apps/desktop/src/components/right-panel/components/chat-model-info-modal.tsx(2 hunks)apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(7 hunks)apps/desktop/src/components/right-panel/components/chat/index.ts(1 hunks)apps/desktop/src/components/right-panel/components/chat/markdown-card.tsx(2 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(1 hunks)packages/ui/src/components/chat/chat-history-view.tsx(1 hunks)packages/ui/src/components/chat/chat-messages-view.tsx(1 hunks)packages/ui/src/components/chat/chat-model-info-modal.tsx(1 hunks)packages/ui/src/components/chat/empty-chat-state.tsx(1 hunks)packages/ui/src/components/chat/markdown-card.tsx(1 hunks)packages/ui/src/components/chat/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/components/chat/empty-chat-state.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
packages/ui/src/components/chat/chat-messages-view.tsxapps/desktop/src/components/right-panel/components/chat/chat-input.tsxpackages/ui/src/components/chat/chat-model-info-modal.tsxpackages/ui/src/components/chat/types.tspackages/ui/src/components/chat/markdown-card.tsxapps/desktop/src/components/right-panel/components/chat/markdown-card.tsxpackages/ui/src/components/chat/chat-history-view.tsxapps/desktop/src/components/right-panel/components/chat/index.tsapps/desktop/src/components/right-panel/components/chat-model-info-modal.tsxapps/desktop/src/components/right-panel/views/chat-view.tsx
🧬 Code graph analysis (4)
packages/ui/src/components/chat/chat-messages-view.tsx (1)
packages/utils/src/ai.ts (1)
UIMessage(25-25)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (2)
packages/utils/src/contexts/right-panel.tsx (1)
useRightPanel(112-118)apps/desktop/src/components/right-panel/hooks/useChatInput.ts (1)
useChatInput(19-195)
packages/ui/src/components/chat/chat-model-info-modal.tsx (3)
apps/desktop/src/components/right-panel/components/chat-model-info-modal.tsx (1)
ChatModelInfoModal(10-24)packages/ui/src/components/ui/modal.tsx (4)
Modal(23-83)ModalBody(99-105)ModalTitle(130-136)ModalDescription(143-148)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
packages/ui/src/components/chat/markdown-card.tsx (2)
apps/desktop/src/components/right-panel/components/chat/markdown-card.tsx (1)
MarkdownCard(13-30)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
🔇 Additional comments (1)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
48-62: LGTM! Clean refactoring to use hook.The refactoring successfully consolidates data fetching and business logic into the
useChatInputhook, making this component more focused on presentation and editor management. The hook provides a clean interface with computed values (entityTitle,currentModelName) and handlers (handleMentionSearch,processSelection,hookSubmit).
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/components/right-panel/components/chat/chat-messages-view.tsx(1 hunks)apps/desktop/src/components/right-panel/components/chat/index.ts(1 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(4 hunks)packages/ui/src/components/chat/chat-input.tsx(9 hunks)packages/ui/src/components/chat/chat-model-info-modal.tsx(1 hunks)packages/ui/src/components/chat/chat-types.ts(1 hunks)packages/ui/src/components/chat/thinking-indicator.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/right-panel/components/chat/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
packages/ui/src/components/chat/thinking-indicator.tsxpackages/ui/src/components/chat/chat-model-info-modal.tsxpackages/ui/src/components/chat/chat-types.tsapps/desktop/src/components/right-panel/components/chat/chat-messages-view.tsxpackages/ui/src/components/chat/chat-input.tsxapps/desktop/src/components/right-panel/views/chat-view.tsx
🧬 Code graph analysis (2)
packages/ui/src/components/chat/chat-input.tsx (1)
packages/utils/src/contexts/right-panel.tsx (1)
SelectionData(4-10)
apps/desktop/src/components/right-panel/views/chat-view.tsx (1)
apps/desktop/src/components/right-panel/hooks/useChatInput.ts (1)
useChatInput(19-195)
🪛 GitHub Actions: .github/workflows/desktop_ci.yaml
packages/ui/src/components/chat/chat-input.tsx
[error] 4-4: Cannot find module '@hypr/ui/components/chat/chat-types' or its corresponding type declarations. (tsc --noEmit during 'apps/desktop' typecheck)
🔇 Additional comments (1)
packages/ui/src/components/chat/chat-types.ts (1)
1-20: LGTM!The type definitions are clear and well-structured. The re-export pattern effectively centralizes public type access for chat components, making the API surface cleaner for consumers.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/ui/src/components/chat/ui-message.tsx (1)
48-69: XSS risk in fallback paths requires escaping.The fallback paths at lines 64 and 68 inject raw content into Renderer without HTML escaping. This allows script injection if the content contains malicious HTML. This concern was previously flagged and remains unaddressed.
Apply this diff to add HTML escaping:
+const ESCAPE_HTML_MAP: Record<string, string> = { + "&": "&", + "<": "<", + ">": ">", + '"': """, + "'": "'", +}; + +const escapeHtml = (value: string) => + value.replace(/[&<>"']/g, (char) => ESCAPE_HTML_MAP[char] ?? char); + const TextContent: FC<{ content: string; isHtml?: boolean; convertMarkdownToHtml?: (markdown: string) => Promise<string>; }> = ({ content, isHtml, convertMarkdownToHtml }) => { const [displayHtml, setDisplayHtml] = useState<string>(""); useEffect(() => { const processContent = async () => { if (isHtml) { setDisplayHtml(content); return; } // Convert markdown to HTML if (convertMarkdownToHtml) { try { let html = await convertMarkdownToHtml(content); // Clean up empty paragraphs html = html .replace(/<p>\s*<\/p>/g, "") .replace(/<p>\u00A0<\/p>/g, "") .replace(/<p> <\/p>/g, "") .replace(/<p>\s+<\/p>/g, "") .replace(/<p> <\/p>/g, "") .trim(); setDisplayHtml(html); } catch (error) { console.error("Failed to convert markdown:", error); - setDisplayHtml(content); + setDisplayHtml(`<p>${escapeHtml(content)}</p>`); } } else { // Fallback: display as plain text in a paragraph - setDisplayHtml(`<p>${content}</p>`); + setDisplayHtml(`<p>${escapeHtml(content)}</p>`); } };packages/ui/src/components/chat/chat-input.tsx (1)
7-7: Verify chat-types module is properly exported.The import path for
BadgeTypecauses a module resolution failure in the desktop CI. While the relative import./chat-typesis syntactically correct, the error suggests the module isn't being exported from the UI package's public API.Please verify that
chat-types.tsis included in the package's barrel exports (likely inpackages/ui/src/components/chat/index.tsor similar). Run the following to check:#!/bin/bash # Check if chat-types is exported from the UI package rg -n "export.*chat-types" packages/ui/
🧹 Nitpick comments (2)
apps/desktop/src/components/right-panel/components/chat/index.ts (1)
2-2: Redundant export: already covered by wildcard.Line 1's
export * from "./empty-chat-state"already re-exports all named exports from that module, includingEmptyChatState. The explicit export on Line 2 is unnecessary unlessEmptyChatStateis a default export being renamed (which would requireexport { default as EmptyChatState }).As per coding guidelines.
Apply this diff to remove the redundant export:
export * from "./empty-chat-state"; -export { EmptyChatState } from "./empty-chat-state"; export * from "./types";packages/ui/src/components/chat/ui-message.tsx (1)
196-198: Refactor to use proper types instead of 'any'.Using
anytypes at lines 196 and 237 bypasses TypeScript's type safety. Consider using the properUIMessagetypes or creating a more specific type for message parts.Also applies to: 237-237
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src/components/right-panel/components/chat/index.ts(1 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(5 hunks)packages/ui/src/components/chat/chat-input.tsx(10 hunks)packages/ui/src/components/chat/chat-messages-view.tsx(3 hunks)packages/ui/src/components/chat/chat-types.ts(1 hunks)packages/ui/src/components/chat/markdown-card.tsx(3 hunks)packages/ui/src/components/chat/ui-message.tsx(7 hunks)packages/ui/src/components/chat/utils/markdown-parser.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/right-panel/views/chat-view.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
packages/ui/src/components/chat/utils/markdown-parser.tsapps/desktop/src/components/right-panel/components/chat/index.tspackages/ui/src/components/chat/markdown-card.tsxpackages/ui/src/components/chat/ui-message.tsxpackages/ui/src/components/chat/chat-types.tspackages/ui/src/components/chat/chat-input.tsxpackages/ui/src/components/chat/chat-messages-view.tsx
🧬 Code graph analysis (3)
packages/ui/src/components/chat/ui-message.tsx (2)
packages/ui/src/components/chat/utils/markdown-parser.ts (2)
ParsedPart(1-5)parseMarkdownBlocks(7-66)packages/ui/src/components/chat/markdown-card.tsx (1)
MarkdownCard(15-141)
packages/ui/src/components/chat/chat-input.tsx (1)
packages/utils/src/contexts/right-panel.tsx (1)
SelectionData(4-10)
packages/ui/src/components/chat/chat-messages-view.tsx (2)
packages/ui/src/components/chat/utils/markdown-parser.ts (1)
parseMarkdownBlocks(7-66)packages/ui/src/components/chat/markdown-card.tsx (1)
MarkdownCard(15-141)
🔇 Additional comments (5)
packages/ui/src/components/chat/utils/markdown-parser.ts (1)
7-66: LGTM! Solid markdown block parser implementation.The parsing logic correctly handles markdown delimiters, incomplete blocks, and edge cases like consecutive delimiters. The loop boundary condition
i < text.length - 2is appropriate for checking three-character delimiters safely.packages/ui/src/components/chat/markdown-card.tsx (1)
51-51: LGTM! Dependency array is now complete.The addition of
convertMarkdownToHtmlto the dependency array correctly addresses the past review comment about missing dependencies.packages/ui/src/components/chat/chat-types.ts (1)
1-20: LGTM! Well-structured type definitions.The public types are clearly defined and properly re-exported from internal modules. The type definitions support the chat UI refactor objectives.
packages/ui/src/components/chat/chat-messages-view.tsx (1)
1-123: LGTM! Props correctly threaded through component hierarchy.The addition of
convertMarkdownToHtmland its propagation to child components (UIMessageComponent along with parseMarkdownBlocks and MarkdownCard) properly enables the markdown rendering pipeline.packages/ui/src/components/chat/chat-input.tsx (1)
159-175: processSelection is already memoized
processSelectionis defined withuseCallbackin apps/desktop/src/components/right-panel/hooks/useChatInput.ts:141; no action required.Likely an incorrect or invalid review comment.
| interface ParsedPart { | ||
| type: "text" | "markdown"; | ||
| content: string; | ||
| isComplete?: boolean; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Import ParsedPart instead of redefining it.
ParsedPart is already defined and exported from ./utils/markdown-parser. Redefining it here creates unnecessary duplication and potential type mismatches.
Apply this diff:
-// Import these from the same package structure
-// Assuming these exist in the UI package - if not, they need to be moved/created
-interface ParsedPart {
- type: "text" | "markdown";
- content: string;
- isComplete?: boolean;
-}
+import type { ParsedPart } from "./utils/markdown-parser";📝 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.
| interface ParsedPart { | |
| type: "text" | "markdown"; | |
| content: string; | |
| isComplete?: boolean; | |
| } | |
| // Remove the local ParsedPart definition and import the shared type instead | |
| import type { ParsedPart } from "./utils/markdown-parser"; |
🤖 Prompt for AI Agents
In packages/ui/src/components/chat/ui-message.tsx around lines 9 to 13, the file
redefines the ParsedPart interface which is already exported from
./utils/markdown-parser; remove the local interface definition and import the
existing type instead (use an import type { ParsedPart } from
'./utils/markdown-parser'); update any local references to use the imported
ParsedPart and ensure TypeScript-only import if appropriate to avoid runtime
import cost.
3 participants
No description provided.