Conversation
…-fix-certain-part
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds cross-component editor sharing via a global editor ref, threads selectionData/htmlContent through right-panel chat flows, introduces selection→chat tooling (edit_enhanced_note, search tools), adds an AI-highlight TipTap mark with styles, and updates renderer/message rendering to support tool-provided HTML and selection context. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant EA as EditorArea (Tiptap)
participant TP as TextSelectionPopover
participant RP as RightPanelContext
participant CI as ChatInput
participant CL as useChatLogic
participant TL as Tools
participant JT as Jinja Template
U->>EA: Select text
U->>TP: Click "Ask AI"
TP->>EA: Read TipTap positions & DOM HTML/text
TP->>RP: sendSelectionToChat(selectionData)
RP-->>CI: pendingSelection set (open/focus chat)
CI->>CI: Insert non-editable selection-ref (dedupe)
U->>CI: Submit message
CI->>CL: handleSubmit(mentionedContent, selectionData, htmlContent)
CL->>JT: prepareMessageHistory(..., selectionData)
alt selection-driven tool
CL->>TL: createEditEnhancedNoteTool({ sessionId, selectionData })
TL->>EA: Use globalEditorRef, replace range, setAIHighlight, show Undo/Accept
else search tools
CL->>TL: createSearchSessionTool / createSearchSessionDateRangeTool -> results
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
|
||
| if (chunk.type === "tool-result" && !(chunk.toolName === "update_progress" && type === "HyprLocal")) { | ||
| didInitializeAiResponse = false; | ||
| console.log("tool result", chunk); |
There was a problem hiding this comment.
Avoid logging full tool-result chunks in production; use structured logging or remove the debug statement.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/right-panel/hooks/useChatLogic.ts at line 455:
<comment>Avoid logging full tool-result chunks in production; use structured logging or remove the debug statement.</comment>
<file context>
@@ -483,6 +452,7 @@ export function useChatLogic({
if (chunk.type === "tool-result" && !(chunk.toolName === "update_progress" && type === "HyprLocal")) {
didInitializeAiResponse = false;
+ console.log("tool result", chunk);
const toolResultMessage: Message = {
</file context>
| console.log("tool result", chunk); | |
| // removed debug log |
| const abortController = new AbortController(); | ||
| abortControllerRef.current = abortController; | ||
|
|
||
| console.log("selectionData exists?", selectionData); |
There was a problem hiding this comment.
Remove debug logging of selectionData to avoid leaking potentially sensitive user content and reduce noise in production logs.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/right-panel/hooks/useChatLogic.ts at line 303:
<comment>Remove debug logging of selectionData to avoid leaking potentially sensitive user content and reduce noise in production logs.</comment>
<file context>
@@ -283,63 +289,19 @@ export function useChatLogic({
}
}
- const searchTool = tool({
- description:
- "Search for sessions (meeting notes) with multiple keywords. The keywords should be the most important things that the user is talking about. This could be either topics, people, or company names.",
- inputSchema: z.object({
- keywords: z.array(z.string()).min(3).max(5).describe(
- "List of 3-5 keywords to search for, each keyword should be concise",
</file context>
| console.log("selectionData exists?", selectionData); | |
| // removed debug log |
| if (chatInputRef.current) { | ||
| chatInputRef.current.focus(); | ||
| } else { | ||
| setTimeout(focusInput, 50); |
There was a problem hiding this comment.
Unbounded recursive setTimeout retry; add a max retry count or cancel condition to prevent indefinite polling.
Prompt for AI agents
Address the following comment on apps/desktop/src/contexts/right-panel.tsx at line 126:
<comment>Unbounded recursive setTimeout retry; add a max retry count or cancel condition to prevent indefinite polling.</comment>
<file context>
@@ -92,6 +104,51 @@ export function RightPanelProvider({
[isExpanded, currentView],
);
+ const sendSelectionToChat = useCallback((selectionData: SelectionData) => {
+ console.log("🚀 sendSelectionToChat called");
+ console.log("Current panel state:", { isExpanded, currentView });
+ console.log("Selection data:", selectionData);
+
+ setPendingSelection(selectionData);
</file context>
| const sendSelectionToChat = useCallback((selectionData: SelectionData) => { | ||
| console.log("🚀 sendSelectionToChat called"); | ||
| console.log("Current panel state:", { isExpanded, currentView }); | ||
| console.log("Selection data:", selectionData); |
There was a problem hiding this comment.
Logs potentially sensitive SelectionData (text/sessionId) to console; avoid leaking user content to logs in production.
Prompt for AI agents
Address the following comment on apps/desktop/src/contexts/right-panel.tsx at line 110:
<comment>Logs potentially sensitive SelectionData (text/sessionId) to console; avoid leaking user content to logs in production.</comment>
<file context>
@@ -92,6 +104,51 @@ export function RightPanelProvider({
[isExpanded, currentView],
);
+ const sendSelectionToChat = useCallback((selectionData: SelectionData) => {
+ console.log("🚀 sendSelectionToChat called");
+ console.log("Current panel state:", { isExpanded, currentView });
+ console.log("Selection data:", selectionData);
+
+ setPendingSelection(selectionData);
</file context>
| const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note"; | ||
| const selectionRef = `[${noteName} - ${pendingSelection.startOffset}:${pendingSelection.endOffset}]`; | ||
| // Use a mention element with data-mention="true" so Tiptap recognizes it | ||
| const referenceText = `<a class="mention selection-ref" data-mention="true" data-id="selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}" data-type="selection" data-label="${selectionRef}" contenteditable="false">${selectionRef}</a> `; |
There was a problem hiding this comment.
Unescaped user-controlled text is interpolated into an HTML string used by setContent; escape or build the node safely to prevent XSS/injection.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/right-panel/components/chat/chat-input.tsx at line 233:
<comment>Unescaped user-controlled text is interpolated into an HTML string used by setContent; escape or build the node safely to prevent XSS/injection.</comment>
<file context>
@@ -197,27 +206,65 @@ export function ChatInput(
onChange(syntheticEvent);
}
- }, [onSubmit, onChange, extractMentionedContent]);
+ }, [onSubmit, onChange, extractMentionedContent, pendingSelection, clearPendingSelection]);
useEffect(() => {
if (chatInputRef && typeof chatInputRef === "object" && editorRef.current?.editor) {
(chatInputRef as any).current = editorRef.current.editor.view.dom;
</file context>
| description: "Edit a specific part of the enhanced note by replacing HTML content at given ProseMirror positions with new HTML content. Use this when the user asks to modify, change, or replace specific selected content. The selected content is provided as HTML, and you should respond with HTML that maintains proper formatting.", | ||
| inputSchema: z.object({ | ||
| startOffset: z.number().describe("The ProseMirror start position of the content to replace"), | ||
| endOffset: z.number().describe("The ProseMirror end position of the content to replace"), |
There was a problem hiding this comment.
Ensure endOffset is an integer by using z.number().int() in the input schema.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts at line 21:
<comment>Ensure endOffset is an integer by using z.number().int() in the input schema.</comment>
<file context>
@@ -0,0 +1,284 @@
+import type { SelectionData } from "@/contexts/right-panel";
+import { globalEditorRef } from "../../../../shared/editor-ref";
+import { tool } from "@hypr/utils/ai";
+import { z } from "zod";
+
+interface EditEnhancedNoteToolDependencies {
+ sessionId: string | null;
+ sessions: Record<string, any>;
+ selectionData?: SelectionData;
</file context>
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
Show resolved
Hide resolved
| console.log("🔄 Original content saved for undo:", originalContent); | ||
|
|
||
| // Trim and clean the HTML to prevent empty elements | ||
| const cleanedHtml = newHtml.trim().replace(/>\s+</g, '><'); |
There was a problem hiding this comment.
Whitespace-collapsing regex removes significant spaces between inline elements, altering content. Avoid collapsing inter-tag whitespace globally.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts at line 46:
<comment>Whitespace-collapsing regex removes significant spaces between inline elements, altering content. Avoid collapsing inter-tag whitespace globally.</comment>
<file context>
@@ -0,0 +1,284 @@
+import type { SelectionData } from "@/contexts/right-panel";
+import { globalEditorRef } from "../../../../shared/editor-ref";
+import { tool } from "@hypr/utils/ai";
+import { z } from "zod";
+
+interface EditEnhancedNoteToolDependencies {
+ sessionId: string | null;
+ sessions: Record<string, any>;
+ selectionData?: SelectionData;
</file context>
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
Show resolved
Hide resolved
|
|
||
| const searchResults = await Promise.all(searchPromises); | ||
|
|
||
| const combinedResults = new Map(); |
There was a problem hiding this comment.
Use a typed Map with explicit key/value generics to ensure matchedKeywords is type-safe and prevent implicit any.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts at line 26:
<comment>Use a typed Map with explicit key/value generics to ensure matchedKeywords is type-safe and prevent implicit any.</comment>
<file context>
@@ -0,0 +1,59 @@
+import { commands as dbCommands } from "@hypr/plugin-db";
+import { tool } from "@hypr/utils/ai";
+import { z } from "zod";
+
+export const createSearchSessionTool = (userId: string | null) => {
+ return tool({
+ description:
+ "Search for sessions (meeting notes) with multiple keywords. The keywords should be the most important things that the user is talking about. This could be either topics, people, or company names.",
+ inputSchema: z.object({
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/template/assets/chat.system.jinja (1)
1-171: Install and run DPrint to format this fileThe CI failure indicates that the
dprintformatter isn’t available under~/.dprint/bin/dprint. Please ensure the DPrint CLI is installed and on yourPATH, then reformat this template and commit the changes.– Install DPrint (choose one):
• Run the official install script
bash curl -fsSL https://dprint.dev/install.sh | bash
• Or follow the “Formatter” section in our README to install via your package manager.– Once installed, format the file:
dprint fmt --config dprint.json crates/template/assets/chat.system.jinja– Verify there are no unformatted changes before pushing to unblock CI.
🧹 Nitpick comments (30)
packages/tiptap/src/styles/nodes/ai-highlight.css (2)
10-17: Respect reduced motion preferencesDisable the pulse animation for users with reduced-motion enabled.
} + +@media (prefers-reduced-motion: reduce) { + .tiptap-normal mark[data-ai-highlight="true"], + .tiptap mark[data-ai-highlight="true"] { + animation: none !important; + } +} @keyframes ai-highlight-pulse {
2-11: Trim down blanket !importantPrefer selector specificity over !important where possible to reduce CSS override contention, keeping !important only for properties that truly need it (likely just the animation or border in contested contexts).
apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts (5)
9-13: Normalize keywords at the schema boundaryTrim inputs to avoid empty/whitespace-only tokens passing validation.
- keywords: z.array(z.string()).min(3).max(5).describe( + keywords: z.array(z.string().trim().min(1)).min(3).max(5).describe( "List of 3-5 keywords to search for, each keyword should be concise", ),
15-23: Deduplicate and trim keywords before queryingAvoid redundant queries and improve result relevance by normalizing input.
- execute: async ({ keywords }) => { - const searchPromises = keywords.map(keyword => + execute: async ({ keywords }) => { + const normalizedKeywords = Array.from(new Set(keywords.map(k => k.trim()).filter(Boolean))); + const searchPromises = normalizedKeywords.map(keyword => dbCommands.listSessions({ type: "search", query: keyword, user_id: userId || "", limit: 3, }) ); - const searchResults = await Promise.all(searchPromises); + const searchResults = await Promise.all(searchPromises);
26-40: Add types for combinedResults to avoid implicit anysType the Map and derived values for safer access to session.id and matchedKeywords.
- const combinedResults = new Map(); + const combinedResults = new Map<string, SessionWithKeywords>(); - searchResults.forEach((sessions, index) => { - const keyword = keywords[index]; + searchResults.forEach((sessions, index) => { + const keyword = normalizedKeywords[index]; sessions.forEach(session => { - if (combinedResults.has(session.id)) { - combinedResults.get(session.id).matchedKeywords.push(keyword); - } else { - combinedResults.set(session.id, { - ...session, - matchedKeywords: [keyword], - }); - } + const existing = combinedResults.get(session.id); + if (existing) existing.matchedKeywords.push(keyword); + else combinedResults.set(session.id, { ...session, matchedKeywords: [keyword] }); }); });Additional code (outside this hunk) to add near the top of the file after imports:
type SessionArray = Awaited<ReturnType<typeof dbCommands.listSessions>>; type Session = SessionArray extends Array<infer T> ? T : never; type SessionWithKeywords = Session & { matchedKeywords: string[] };
42-47: Preserve types through finalResultsType the array so downstream consumers get session+keywords.
- const finalResults = Array.from(combinedResults.values()) + const finalResults: SessionWithKeywords[] = Array.from(combinedResults.values()) .sort((a, b) => b.matchedKeywords.length - a.matchedKeywords.length);
50-55: Align summary map with runtime object keysObject keys are strings at runtime; use Record<string, number> to reflect that and avoid index type friction.
- }, {} as Record<number, number>), + }, {} as Record<string, number>),packages/tiptap/src/shared/ai-highlight.ts (2)
67-72: Parse only ai-highlight="true" marksTighten the selector to avoid accidentally parsing unrelated marks that merely carry a data-ai-highlight attribute with other values.
- return [{ - tag: 'mark[data-ai-highlight]', + return [{ + tag: 'mark[data-ai-highlight="true"]', priority: 100, // Higher priority to ensure it's recognized }];
31-35: Unused CSS class?You render class "ai-generated-highlight" but the CSS targets only [data-ai-highlight="true"]. Either add a rule for the class or drop the class to reduce DOM noise.
apps/desktop/src/components/right-panel/views/chat-view.tsx (1)
183-183: Pass handler directly to avoid needless lambda allocation.Minor cleanup; keeps props stable and helps memoization.
- onSubmit={(mentionedContent, selectionData) => handleSubmit(mentionedContent, selectionData)} + onSubmit={handleSubmit}crates/template/assets/chat.user.jinja (1)
2-8: Cap selection payload to avoid unbounded token usageCurrently,
chat-utils.tsforwardsselectionData.textstraight into the Jinja template with no length checks, which can lead to prompt bloat (and unpredictable token costs) when users select very large note fragments. To keep token usage predictable, consider truncating or chunking the selection before passing it into the template.• Location to address:
- File:
apps/desktop/src/components/right-panel/utils/chat-utils.ts(around lines 203–207)- File:
crates/template/assets/chat.user.jinja(lines 5–8)• Suggested refactor (optional):
- In
chat-utils.ts, introduce a maximum‐length cap (e.g. 2000 characters) or split largeselectionData.textinto manageable chunks.- Pass the truncated or chunked text into the template so that the Jinja snippet receives only the portion you intend to process:
selectionData: selectionData ? { - text: selectionData.text, + text: truncate(selectionData.text, MAX_SELECTION_LENGTH), startOffset: selectionData.startOffset, endOffset: selectionData.endOffset, } : undefined,This ensures the engine never receives an unexpectedly large payload, keeps token counts predictable, and prevents potential service slowdowns.
apps/desktop/src/shared/editor-ref.ts (1)
8-10: Global mutable ref needs session scoping to avoid cross-tab/editor mixups.At minimum, attach sessionId and provide attach/detach helpers so tools can validate the target editor.
-export const globalEditorRef = { - current: null as TiptapEditor | null -}; +export const globalEditorRef = { + current: null as TiptapEditor | null, + sessionId: undefined as string | undefined, +};Outside this range, consider adding:
export function attachEditor(editor: TiptapEditor, sessionId: string) { globalEditorRef.current = editor; globalEditorRef.sessionId = sessionId; } export function detachEditor(editor: TiptapEditor) { if (globalEditorRef.current === editor) { globalEditorRef.current = null; globalEditorRef.sessionId = undefined; } } export function getEditorForSession(sessionId: string) { return globalEditorRef.sessionId === sessionId ? globalEditorRef.current : null; }apps/desktop/src/contexts/ai-edit.tsx (2)
1-1: Memoize provider value to avoid unnecessary rerenders.Prevents child tree rerender on every state change unrelated to value identity.
-import { createContext, useContext, useState } from "react"; +import { createContext, useContext, useMemo, useState } from "react";
21-25: Memoize context value.Keeps reference stable for consumers using referential equality.
- return ( - <AIEditContext.Provider value={{ currentAIEdit, setCurrentAIEdit }}> - {children} - </AIEditContext.Provider> - ); + const value = useMemo(() => ({ currentAIEdit, setCurrentAIEdit }), [currentAIEdit]); + return <AIEditContext.Provider value={value}>{children}</AIEditContext.Provider>;apps/desktop/src/components/editor-area/index.tsx (1)
133-145: Prefer callback ref over effect for global editor registrationAvoid effect deps on ref internals and register/unregister atomically via a callback ref.
Apply this diff to remove the effect:
- // Assign editor to global ref for access by other components (like chat tools) - useEffect(() => { - if (editorRef.current?.editor) { - globalEditorRef.current = editorRef.current.editor; - } - // Clear on unmount - return () => { - if (globalEditorRef.current === editorRef.current?.editor) { - globalEditorRef.current = null; - } - }; - }, [editorRef.current?.editor]);Then update the Editor ref usage outside this range:
// inside JSX <Editor key={editorKey} ref={(inst) => { (editorRef as any).current = inst; globalEditorRef.current = inst?.editor ?? null; }} /* ... */ /> // and for Renderer <Renderer ref={(inst) => { (editorRef as any).current = inst; globalEditorRef.current = inst?.editor ?? null; }} /* ... */ />apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (4)
292-299: Tool factory usage is clean; minor guard suggestionConsider constructing edit_enhanced_note only when selectionData exists to avoid carrying unused closures.
- const editEnhancedNoteTool = createEditEnhancedNoteTool({ - sessionId, - sessions, - selectionData, - }); + const editEnhancedNoteTool = selectionData + ? createEditEnhancedNoteTool({ sessionId, sessions, selectionData }) + : undefined;
303-304: Remove debug logs that can leak PIIselectionData may include user content; avoid logging it in production.
- console.log("selectionData exists?", selectionData); + if (import.meta.env.DEV) console.log("selectionData exists?", !!selectionData);
455-456: Drop verbose tool result loggingAvoid logging full tool payloads/results; they may contain sensitive content.
- console.log("tool result", chunk); + if (import.meta.env.DEV) console.log("tool finished:", chunk.toolName);
520-571: Close MCP clients on error path, not just onFinishCurrently clients are closed only in onFinish; add cleanup in the catch as well.
- } catch (error) { + } catch (error) { console.error(error); + try { + for (const client of allMcpClients) client.close(); + hyprMcpClient?.close(); + } catch {}apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
213-213: Remove noisy console log of rendered user messageRendered prompt can include sensitive notes; don’t log it.
- console.log("this is user message", userContent); + if (import.meta.env.DEV) console.log("user message prepared");apps/desktop/src/components/editor-area/text-selection-popover.tsx (4)
8-15: Remove unused onAskAI propIt’s no longer used; drop it from the interface and component props to satisfy “no unused” guideline.
interface TextSelectionPopoverProps { isEnhancedNote: boolean; onAnnotate: (selectedText: string, selectedRect: DOMRect) => void; - onAskAI?: (selectedText: string) => void; isAnnotationBoxOpen: boolean; sessionId: string; - editorRef: React.RefObject<{ editor: any }>; + editorRef: React.RefObject<{ editor: unknown }>; }And in the component signature:
-export function TextSelectionPopover( - { isEnhancedNote, onAnnotate, onAskAI, isAnnotationBoxOpen, sessionId, editorRef }: TextSelectionPopoverProps, +export function TextSelectionPopover( + { isEnhancedNote, onAnnotate, isAnnotationBoxOpen, sessionId, editorRef }: TextSelectionPopoverProps,
25-27: Avoid NodeJS.Timeout in browser codeUse ReturnType for portability.
- const delayTimeoutRef = useRef<NodeJS.Timeout>(); + const delayTimeoutRef = useRef<ReturnType<typeof setTimeout>>();
110-152: Clarify payload semantics: this is HTML, not plain textThe field is called text but holds HTML; either rename or document clearly to avoid misuse downstream.
1-254: Trim debug logsReduce console noise; keep only gated logs.
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (2)
103-110: Avoid innerHTML; build buttons safelyStatic SVG/labels are fine, but using DOM APIs avoids future XSS lint noise.
- const undoBtn = document.createElement('button'); - undoBtn.innerHTML = ` - <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"> - <path d="M3 7v6h6"/><path d="M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13"/> - </svg> - <span style="margin-left: 4px; font-size: 11px;">Undo</span> - `; + const undoBtn = document.createElement('button'); + const undoSvg = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); + undoSvg.setAttribute('width','12'); undoSvg.setAttribute('height','12'); + undoSvg.setAttribute('viewBox','0 0 24 24'); undoSvg.setAttribute('fill','none'); + undoSvg.setAttribute('stroke','currentColor'); undoSvg.setAttribute('stroke-width','2'); + const p1 = document.createElementNS('http://www.w3.org/2000/svg','path'); + p1.setAttribute('d','M3 7v6h6'); + const p2 = document.createElementNS('http://www.w3.org/2000/svg','path'); + p2.setAttribute('d','M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13'); + undoSvg.append(p1,p2); + const undoLabel = document.createElement('span'); + undoLabel.style.marginLeft = '4px'; + undoLabel.style.fontSize = '11px'; + undoLabel.textContent = 'Undo'; + undoBtn.append(undoSvg, undoLabel);
148-155: Same for Accept button: avoid innerHTMLMirror the approach used for Undo.
- const acceptBtn = document.createElement('button'); - acceptBtn.innerHTML = ` - <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="white" stroke-width="2"> - <polyline points="20 6 9 17 4 12"/> - </svg> - <span style="margin-left: 4px; font-size: 11px;">Accept</span> - `; + const acceptBtn = document.createElement('button'); + const okSvg = document.createElementNS('http://www.w3.org/2000/svg','svg'); + okSvg.setAttribute('width','12'); okSvg.setAttribute('height','12'); + okSvg.setAttribute('viewBox','0 0 24 24'); okSvg.setAttribute('fill','none'); + okSvg.setAttribute('stroke','white'); okSvg.setAttribute('stroke-width','2'); + const pl = document.createElementNS('http://www.w3.org/2000/svg','polyline'); + pl.setAttribute('points','20 6 9 17 4 12'); + okSvg.append(pl); + const okLabel = document.createElement('span'); + okLabel.style.marginLeft = '4px'; + okLabel.style.fontSize = '11px'; + okLabel.textContent = 'Accept'; + acceptBtn.append(okSvg, okLabel);apps/desktop/src/contexts/right-panel.tsx (2)
121-130: Refactor duplicated focus logic using imported utilityThe focus logic is duplicated with the same pattern appearing elsewhere in the file. Consider using the existing
focusInpututility fromchat-utils.ts.Import the utility function at the top of the file:
import { focusInput } from "@/components/right-panel/utils/chat-utils";Then simplify the focus logic:
setTimeout(() => { - const focusInput = () => { - if (chatInputRef.current) { - chatInputRef.current.focus(); - } else { - setTimeout(focusInput, 50); - } - }; - focusInput(); + focusInput(chatInputRef); }, 350);Note: The existing utility may need to be enhanced to support the retry logic if it doesn't already.
10-16: Consider more descriptive field names for ProseMirror positionsThe current field names
startOffsetandendOffsetcould be misleading since they represent ProseMirror positions rather than character offsets. Consider renaming them to be more explicit.export interface SelectionData { text: string; - startOffset: number; // ProseMirror position (not character offset) - endOffset: number; // ProseMirror position (not character offset) + startPos: number; // ProseMirror absolute position + endPos: number; // ProseMirror absolute position sessionId: string; timestamp: number; }apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (2)
140-141: Consider a more robust deduplication mechanismUsing a string concatenation for deduplication could have edge cases. Consider using a more robust approach.
const editorRef = useRef<{ editor: TiptapEditor | null }>(null); -const processedSelectionRef = useRef<string | null>(null); +const processedSelectionRef = useRef<SelectionData | null>(null);Then update the comparison logic:
-const selectionId = `${pendingSelection.startOffset}-${pendingSelection.endOffset}-${pendingSelection.timestamp}`; - -// Only process if we haven't already processed this exact selection -if (processedSelectionRef.current !== selectionId) { +// Only process if we haven't already processed this exact selection +if (!processedSelectionRef.current || + processedSelectionRef.current.startOffset !== pendingSelection.startOffset || + processedSelectionRef.current.endOffset !== pendingSelection.endOffset || + processedSelectionRef.current.timestamp !== pendingSelection.timestamp) {And update the marking:
-processedSelectionRef.current = selectionId; +processedSelectionRef.current = pendingSelection;
346-377: Consider extracting selection-ref styles to a separate CSS fileThe inline styles are growing complex. Consider moving them to a dedicated CSS module for better maintainability.
Create a new file
chat-input.module.cssand move the styles there, then import and use CSS modules for better organization and potential performance benefits.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
apps/desktop/src/components/editor-area/index.tsx(4 hunks)apps/desktop/src/components/editor-area/text-selection-popover.tsx(4 hunks)apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(8 hunks)apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(8 hunks)apps/desktop/src/components/right-panel/utils/chat-utils.ts(3 hunks)apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts(1 hunks)apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts(1 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(1 hunks)apps/desktop/src/contexts/ai-edit.tsx(1 hunks)apps/desktop/src/contexts/right-panel.tsx(5 hunks)apps/desktop/src/shared/editor-ref.ts(1 hunks)crates/template/assets/chat.system.jinja(1 hunks)crates/template/assets/chat.user.jinja(1 hunks)packages/tiptap/src/shared/ai-highlight.ts(1 hunks)packages/tiptap/src/shared/extensions.ts(2 hunks)packages/tiptap/src/styles/nodes/ai-highlight.css(1 hunks)packages/tiptap/src/styles/tiptap.css(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/utils/tools/search_session_multi_keywords.tspackages/tiptap/src/shared/ai-highlight.tsapps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.tsapps/desktop/src/components/right-panel/views/chat-view.tsxpackages/tiptap/src/shared/extensions.tsapps/desktop/src/contexts/right-panel.tsxapps/desktop/src/shared/editor-ref.tsapps/desktop/src/components/right-panel/components/chat/chat-input.tsxapps/desktop/src/contexts/ai-edit.tsxapps/desktop/src/components/editor-area/text-selection-popover.tsxapps/desktop/src/components/right-panel/utils/chat-utils.tsapps/desktop/src/components/right-panel/hooks/useChatLogic.tsapps/desktop/src/components/editor-area/index.tsx
🧬 Code graph analysis (9)
apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts (1)
packages/utils/src/ai.ts (1)
tool(18-18)
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (3)
apps/desktop/src/contexts/right-panel.tsx (1)
SelectionData(10-16)packages/utils/src/ai.ts (1)
tool(18-18)apps/desktop/src/shared/editor-ref.ts (1)
globalEditorRef(8-10)
packages/tiptap/src/shared/extensions.ts (1)
packages/tiptap/src/shared/ai-highlight.ts (1)
AIHighlight(26-91)
apps/desktop/src/contexts/right-panel.tsx (1)
apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
focusInput(30-34)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
apps/desktop/src/contexts/right-panel.tsx (2)
SelectionData(10-16)useRightPanel(237-243)
apps/desktop/src/components/editor-area/text-selection-popover.tsx (2)
apps/desktop/src/contexts/right-panel.tsx (1)
useRightPanel(237-243)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
apps/desktop/src/contexts/right-panel.tsx (1)
SelectionData(10-16)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (3)
apps/desktop/src/contexts/right-panel.tsx (1)
SelectionData(10-16)apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts (1)
createSearchSessionTool(5-59)apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (1)
createEditEnhancedNoteTool(12-284)
apps/desktop/src/components/editor-area/index.tsx (1)
apps/desktop/src/shared/editor-ref.ts (1)
globalEditorRef(8-10)
🪛 GitHub Actions: .github/workflows/fmt.yaml
packages/tiptap/src/shared/ai-highlight.ts
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
crates/template/assets/chat.system.jinja
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
packages/tiptap/src/styles/nodes/ai-highlight.css
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/contexts/right-panel.tsx
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/shared/editor-ref.ts
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/contexts/ai-edit.tsx
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/components/editor-area/text-selection-popover.tsx
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
apps/desktop/src/components/editor-area/index.tsx
[error] 1-1: dprint: not formatted according to configuration. Command: ~/.dprint/bin/dprint check --config 'dprint.json'. Run 'dprint --write' to fix formatting.
🪛 ast-grep (0.38.6)
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
[warning] 103-108: 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: undoBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"> <path d="M3 7v6h6"/><path d="M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Undo</span>
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] 148-153: 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: acceptBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="white" stroke-width="2"> <polyline points="20 6 9 17 4 12"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Accept</span>
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] 103-108: 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: undoBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"> <path d="M3 7v6h6"/><path d="M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Undo</span>
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] 148-153: 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: acceptBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="white" stroke-width="2"> <polyline points="20 6 9 17 4 12"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Accept</span>
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). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (22)
packages/tiptap/src/styles/nodes/ai-highlight.css (1)
1-17: dprint formatter failure: CLI not foundIt looks like the dprint binary isn’t available at
~/.dprint/bin/dprint, so your formatting step can’t run and CI will continue to fail. Please:
- Install the dprint CLI on your machine/CI agent. For example:
•npm install -g dprint
• orpnpm add -D dprint- Run the formatter using a local or project‐scoped install instead of a hard-coded path. For example:
•npx dprint fmt packages/tiptap/src/styles/nodes/ai-highlight.css
•pnpm exec dprint fmt packages/tiptap/src/styles/nodes/ai-highlight.css- Verify formatting is clean with
dprint check(ornpx dprint check/pnpm exec dprint check) and commit any changes.Once the CLI is installed or the invocation is updated, re-run the formatter to unblock CI.
packages/tiptap/src/styles/tiptap.css (1)
13-13: Import placement LGTMImporting ai-highlight.css right after mark.css is appropriate for mark-level styling precedence.
apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts (1)
19-20: No Cross-User Leak; emptyuser_idsimply yields no results
I’ve confirmed that in the generated TypeScript bindings, thelistSessionsfilter always requires a non-optionaluser_id: string(neverundefinedornull) — seeListSessionFilterinpackages/obsidian/generated/types.gen.ts. On the Rust side, the DB plugin passes this exact string into a SQLWHERE s.user_id = ?query (viacrates/db-user/src/sessions_ops.rs), so supplying""will simply match no sessions rather than falling back to a global/default user . There’s no semantic distinction in the plugin between an empty string and any other string: it always filters by exactly what you pass.If you prefer to avoid calling
listSessionswith an empty ID entirely, you can add a guard in the TS code (e.g.if (!userId) return [];), but functionally there’s no risk of leaking another user’s sessions.packages/tiptap/src/shared/ai-highlight.ts (1)
1-1: Run dprint to format ai-highlight.tsThis file is flagged by dprint. Please ensure you have dprint available (e.g. installed in devDependencies or via your package manager), then run the formatter and verification steps:
# Install dprint if needed npm install --save-dev dprint # Format the file and apply quote‐style normalization npx dprint fmt packages/tiptap/src/shared/ai-highlight.ts # Verify no other files are out of compliance npx dprint checkThis will enforce the repo’s formatting conventions (including consistent quote style) across your changes.
packages/tiptap/src/shared/extensions.ts (1)
88-88: Extension registration LGTMAIHighlight is correctly added after Highlight, which is a sensible order for mark precedence.
apps/desktop/src/components/right-panel/views/chat-view.tsx (1)
183-183: onSubmit/handleSubmit signatures match — no changes neededI’ve verified that:
- ChatInputProps defines
onSubmit: (mentionedContent?: Array<{ id: string; type: string; label: string }>, selectionData?: SelectionData) => void
in
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx- The
ChatInputinvocation in
apps/desktop/src/components/right-panel/views/chat-view.tsx
passes
(mentionedContent, selectionData) => handleSubmit(mentionedContent, selectionData)handleSubmitfromuseChatLogicis declared as
const handleSubmit = async (mentionedContent?: Array<{ id: string; type: string; label: string }>, selectionData?: SelectionData) => { … }
in
apps/desktop/src/components/right-panel/hooks/useChatLogic.tsAll parameter types and optionality align across the prop definition, the caller, and the handler implementation.
apps/desktop/src/shared/editor-ref.ts (1)
1-11: Fix dprint formatting to unblock CIIt looks like the CI is failing on styling checks for this file (see .github/workflows/fmt.yaml), but we don’t yet have the
dprintCLI installed locally to repro and fix it. To resolve:
- Install dprint as a dev dependency:
• npm:npm install --save-dev dprint
• pnpm:pnpm add -D dprint- Add a helper script in your package.json:
"scripts": { "format": "dprint fmt --config dprint.json" }- Run the formatter against this file:
npm run format apps/desktop/src/shared/editor-ref.ts- Commit the updated file so that the CI
dprint/checkstep passes.Once formatted, the CI should unblock.
apps/desktop/src/contexts/ai-edit.tsx (1)
1-34: Script requested; awaiting results.apps/desktop/src/components/editor-area/index.tsx (2)
304-306: Propagating sessionId/editorRef to popover looks goodThis aligns the selection-to-chat flow; no issues spotted.
1-341: Fix formatting to unblock CIdprint is failing for this PR. Run: dprint --write
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (3)
121-126: Signature extension to carry selectionData is soundPlumbing selection context into chat is correct and backward-compatible.
322-329: Nice: conditional tool exposure based on selectionDataExposing edit_enhanced_note only when relevant is the right call.
1-611: Fix formatting to unblock CIdprint is failing for this PR. Run: dprint --write
apps/desktop/src/components/right-panel/utils/chat-utils.ts (3)
36-47: Extending prepareMessageHistory with selectionData is correctType-only import and optional param preserve compatibility.
204-211: Good: selectionData serialized as a plain objectThis keeps templates JsonValue-safe.
1-223: Fix formatting to unblock CIdprint is failing for this PR. Run: dprint --write
apps/desktop/src/components/editor-area/text-selection-popover.tsx (3)
153-196: Great UX: robust selection capture and routing to chatSelection extraction, ProseMirror positions, and dispatch via sendSelectionToChat look solid.
230-251: Minor: consistent small-button height class already appliedUI tweaks LGTM.
1-254: Fix formatting to unblock CIdprint is failing for this PR. Run: dprint --write
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (2)
69-76: Good: highlight metadata trackedUsing timestamp/sessionId in the mark aids auditability.
1-285: Fix formatting to unblock CIdprint is failing for this PR. Run: dprint --write
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
261-264: Confirmed.mention-containerselector is correctThe
.mention-containerclass is defined inpackages/tiptap/src/styles/mention.cssand applied to the dropdown wrapper inpackages/tiptap/src/editor/mention.tsx. It matches the element you’re querying inchat-input.tsx, so no changes are needed.
| console.log("💬 Chat Input Received NEW Selection (ProseMirror):"); | ||
| console.log("Pending selection:", pendingSelection); | ||
| console.log("ProseMirror positions:", pendingSelection.startOffset, "to", pendingSelection.endOffset); | ||
|
|
||
| // Create compact reference instead of full quoted text | ||
| const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note"; | ||
| const selectionRef = `[${noteName} - ${pendingSelection.startOffset}:${pendingSelection.endOffset}]`; | ||
| // Use a mention element with data-mention="true" so Tiptap recognizes it | ||
| const referenceText = `<a class="mention selection-ref" data-mention="true" data-id="selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}" data-type="selection" data-label="${selectionRef}" contenteditable="false">${selectionRef}</a> `; | ||
| console.log("Generated selection reference:", referenceText); | ||
|
|
||
| editorRef.current.editor.commands.setContent(referenceText); | ||
| editorRef.current.editor.commands.focus('end'); | ||
|
|
||
| // Clear the input value to match editor content | ||
| const syntheticEvent = { | ||
| target: { value: selectionRef }, | ||
| currentTarget: { value: selectionRef }, | ||
| } as React.ChangeEvent<HTMLTextAreaElement>; | ||
| onChange(syntheticEvent); | ||
| console.log("Chat input populated with selection reference"); |
There was a problem hiding this comment.
Remove console.log statements from selection processing
Multiple console.log statements should be removed before merging to production.
// Only process if we haven't already processed this exact selection
if (processedSelectionRef.current !== selectionId) {
- console.log("💬 Chat Input Received NEW Selection (ProseMirror):");
- console.log("Pending selection:", pendingSelection);
- console.log("ProseMirror positions:", pendingSelection.startOffset, "to", pendingSelection.endOffset);
// Create compact reference instead of full quoted text
const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note";
const selectionRef = `[${noteName} - ${pendingSelection.startOffset}:${pendingSelection.endOffset}]`;
// Use a mention element with data-mention="true" so Tiptap recognizes it
const referenceText = `<a class="mention selection-ref" data-mention="true" data-id="selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}" data-type="selection" data-label="${selectionRef}" contenteditable="false">${selectionRef}</a> `;
- console.log("Generated selection reference:", referenceText);
editorRef.current.editor.commands.setContent(referenceText);
editorRef.current.editor.commands.focus('end');
// Clear the input value to match editor content
const syntheticEvent = {
target: { value: selectionRef },
currentTarget: { value: selectionRef },
} as React.ChangeEvent<HTMLTextAreaElement>;
onChange(syntheticEvent);
- console.log("Chat input populated with selection reference");
// Mark this selection as processed
processedSelectionRef.current = selectionId;
}🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/components/chat/chat-input.tsx around
lines 225 to 245, remove all console.log(...) debugging statements in the
selection processing block (the four logs before generating the selectionRef,
the log after generating selectionRef, and the final log after onChange) so no
console output remains in production; keep the selectionRef creation,
editor.commands.setContent/focus, syntheticEvent creation, and onChange call
intact and unchanged.
| const noteName = noteData?.title || humanData?.full_name || organizationData?.name || "Note"; | ||
| const selectionRef = `[${noteName} - ${pendingSelection.startOffset}:${pendingSelection.endOffset}]`; | ||
| // Use a mention element with data-mention="true" so Tiptap recognizes it | ||
| const referenceText = `<a class="mention selection-ref" data-mention="true" data-id="selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}" data-type="selection" data-label="${selectionRef}" contenteditable="false">${selectionRef}</a> `; |
There was a problem hiding this comment.
Potential XSS vulnerability with HTML string concatenation
Constructing HTML strings with user data could lead to XSS vulnerabilities. The selectionRef contains data that could potentially be manipulated.
Consider using TipTap's API to insert nodes programmatically instead of HTML strings:
const mentionNode = {
type: 'mention',
attrs: {
'data-mention': 'true',
'data-id': `selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}`,
'data-type': 'selection',
'data-label': selectionRef,
contenteditable: 'false'
},
content: [{ type: 'text', text: selectionRef }]
};
editorRef.current.editor.commands.insertContent(mentionNode);| startOffset: z.number().describe("The ProseMirror start position of the content to replace"), | ||
| endOffset: z.number().describe("The ProseMirror end position of the content to replace"), | ||
| newHtml: z.string().describe("The new HTML content to replace the selected content with. Maintain proper HTML structure and formatting."), | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate and constrain tool inputs
Harden start/end offsets and HTML length to prevent bad ranges or huge payloads.
- inputSchema: z.object({
- startOffset: z.number().describe("The ProseMirror start position of the content to replace"),
- endOffset: z.number().describe("The ProseMirror end position of the content to replace"),
- newHtml: z.string().describe("The new HTML content to replace the selected content with. Maintain proper HTML structure and formatting."),
- }),
+ inputSchema: z.object({
+ startOffset: z.number().int().nonnegative().describe("ProseMirror start position (inclusive)"),
+ endOffset: z.number().int().nonnegative().describe("ProseMirror end position (exclusive)"),
+ newHtml: z.string().min(1).max(20000).describe("Replacement HTML; must be well-formed."),
+ }).refine((v) => v.endOffset > v.startOffset, "endOffset must be greater than startOffset"),📝 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.
| startOffset: z.number().describe("The ProseMirror start position of the content to replace"), | |
| endOffset: z.number().describe("The ProseMirror end position of the content to replace"), | |
| newHtml: z.string().describe("The new HTML content to replace the selected content with. Maintain proper HTML structure and formatting."), | |
| }), | |
| inputSchema: z.object({ | |
| startOffset: z.number().int().nonnegative().describe("ProseMirror start position (inclusive)"), | |
| endOffset: z.number().int().nonnegative().describe("ProseMirror end position (exclusive)"), | |
| newHtml: z.string().min(1).max(20000).describe("Replacement HTML; must be well-formed."), | |
| }).refine((v) => v.endOffset > v.startOffset, "endOffset must be greater than startOffset"), |
| // Capture original content from selectionData BEFORE making any changes | ||
| const originalContent = selectionData?.text || editor.state.doc.textBetween(startOffset, endOffset); | ||
| console.log("🔄 Original content saved for undo:", originalContent); | ||
|
|
||
| // Trim and clean the HTML to prevent empty elements | ||
| const cleanedHtml = newHtml.trim().replace(/>\s+</g, '><'); | ||
|
|
There was a problem hiding this comment.
Sanitize LLM-provided HTML before insert
Insertions come from the model; sanitize to prevent malicious markup slipping through.
- // Trim and clean the HTML to prevent empty elements
- const cleanedHtml = newHtml.trim().replace(/>\s+</g, '><');
+ // Trim and sanitize HTML
+ const raw = newHtml.trim().replace(/>\s+</g, "><");
+ // Use a sanitizer available in the app (e.g., DOMPurify) if present
+ const cleanedHtml = (window as any).DOMPurify
+ ? (window as any).DOMPurify.sanitize(raw, { USE_PROFILES: { html: true } })
+ : raw;If DOMPurify isn’t bundled, I can wire it up or switch to an allowlist-based serializer.
📝 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.
| // Capture original content from selectionData BEFORE making any changes | |
| const originalContent = selectionData?.text || editor.state.doc.textBetween(startOffset, endOffset); | |
| console.log("🔄 Original content saved for undo:", originalContent); | |
| // Trim and clean the HTML to prevent empty elements | |
| const cleanedHtml = newHtml.trim().replace(/>\s+</g, '><'); | |
| // Capture original content from selectionData BEFORE making any changes | |
| const originalContent = selectionData?.text || editor.state.doc.textBetween(startOffset, endOffset); | |
| console.log("🔄 Original content saved for undo:", originalContent); | |
| // Trim and sanitize HTML | |
| const raw = newHtml.trim().replace(/>\s+</g, "><"); | |
| // Use a sanitizer available in the app (e.g., DOMPurify) if present | |
| const cleanedHtml = (window as any).DOMPurify | |
| ? (window as any).DOMPurify.sanitize(raw, { USE_PROFILES: { html: true } }) | |
| : raw; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
around lines 41 to 47, the code trims and collapses whitespace in newHtml but
does not sanitize model-provided HTML; update the flow to pass cleanedHtml
through a trusted HTML sanitizer (prefer DOMPurify if available: import
DOMPurify, run DOMPurify.sanitize(cleanedHtml, {ALLOWED_TAGS, ALLOWED_ATTR})
with a strict allowlist, and use the sanitized output for insertion), and add a
fallback path that uses an allowlist-based serializer if DOMPurify isn’t bundled
(or throw/abort insertion if no sanitizer is configured). Ensure imports and
types are added, and that sanitized HTML replaces cleanedHtml before any DOM
insertion or editor transaction.
| // Delete old content and insert new content with proper parse options | ||
| editor.chain() | ||
| .focus() | ||
| .setTextSelection({ from: startOffset, to: endOffset }) | ||
| .deleteSelection() | ||
| .insertContent(cleanedHtml, { | ||
| parseOptions: { | ||
| preserveWhitespace: false | ||
| } | ||
| }) | ||
| .run(); | ||
|
|
||
| // Calculate actual inserted content size | ||
| const finalDocSize = editor.state.doc.content.size; | ||
| const insertedSize = finalDocSize - initialDocSize + (endOffset - startOffset); | ||
| const highlightEnd = startOffset + insertedSize; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clamp selection to document bounds
Avoid runtime errors when positions are out of bounds.
- editor.chain()
+ const docSize = editor.state.doc.content.size;
+ const from = Math.max(0, Math.min(startOffset, docSize));
+ const to = Math.max(from, Math.min(endOffset, docSize));
+ editor.chain()
.focus()
- .setTextSelection({ from: startOffset, to: endOffset })
+ .setTextSelection({ from, to })
.deleteSelection()
.insertContent(cleanedHtml, {
parseOptions: {
preserveWhitespace: false
}
})
.run();
-
+ // Recompute doc size after changes
// Calculate actual inserted content size
- const finalDocSize = editor.state.doc.content.size;
- const insertedSize = finalDocSize - initialDocSize + (endOffset - startOffset);
- const highlightEnd = startOffset + insertedSize;
+ const finalDocSize = editor.state.doc.content.size;
+ const insertedSize = finalDocSize - initialDocSize + (to - from);
+ const highlightEnd = from + insertedSize;📝 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.
| // Delete old content and insert new content with proper parse options | |
| editor.chain() | |
| .focus() | |
| .setTextSelection({ from: startOffset, to: endOffset }) | |
| .deleteSelection() | |
| .insertContent(cleanedHtml, { | |
| parseOptions: { | |
| preserveWhitespace: false | |
| } | |
| }) | |
| .run(); | |
| // Calculate actual inserted content size | |
| const finalDocSize = editor.state.doc.content.size; | |
| const insertedSize = finalDocSize - initialDocSize + (endOffset - startOffset); | |
| const highlightEnd = startOffset + insertedSize; | |
| // Delete old content and insert new content with proper parse options | |
| const docSize = editor.state.doc.content.size; | |
| const from = Math.max(0, Math.min(startOffset, docSize)); | |
| const to = Math.max(from, Math.min(endOffset, docSize)); | |
| editor.chain() | |
| .focus() | |
| .setTextSelection({ from, to }) | |
| .deleteSelection() | |
| .insertContent(cleanedHtml, { | |
| parseOptions: { | |
| preserveWhitespace: false | |
| } | |
| }) | |
| .run(); | |
| // Recompute doc size after changes | |
| // Calculate actual inserted content size | |
| const finalDocSize = editor.state.doc.content.size; | |
| const insertedSize = finalDocSize - initialDocSize + (to - from); | |
| const highlightEnd = from + insertedSize; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
around lines 51 to 67, clamp startOffset and endOffset to valid document bounds
before calling editor.chain(): ensure startOffset = Math.max(0,
Math.min(startOffset, editor.state.doc.content.size)) and endOffset =
Math.max(startOffset, Math.min(endOffset, editor.state.doc.content.size)) (or
equivalent) so selection positions cannot be out of range; use the clamped
values for setTextSelection, delete/insert operations and for computing
finalDocSize/insertedSize/highlightEnd to avoid runtime out-of-bounds errors.
| - edit_enhanced_note: Only call when you are given a certain Prosemirror range of text that the user wants to edit. Replace that certain range with a new text that is written in a way | ||
| that the user wants. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten tool contract and forbid raw-HTML chat responses; require selectionData/session gating.
Strengthen the invocation rules so the model edits only via the tool and never emits free-form HTML in chat.
- - edit_enhanced_note: Only call when you are given a certain Prosemirror range of text that the user wants to edit. Replace that certain range with a new text that is written in a way
- that the user wants.
+ - edit_enhanced_note: Call only when selectionData is present (must include startOffset, endOffset, sessionId) and the active editor is available. Provide an HTML fragment that replaces exactly that range, preserving nodes/marks. Do NOT output HTML in normal chat responses—perform edits exclusively via this tool.📝 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.
| - edit_enhanced_note: Only call when you are given a certain Prosemirror range of text that the user wants to edit. Replace that certain range with a new text that is written in a way | |
| that the user wants. | |
| - edit_enhanced_note: Call only when selectionData is present (must include startOffset, endOffset, sessionId) and the active editor is available. Provide an HTML fragment that replaces exactly that range, preserving nodes/marks. Do NOT output HTML in normal chat responses—perform edits exclusively via this tool. |
🤖 Prompt for AI Agents
In crates/template/assets/chat.system.jinja around lines 163-165, tighten the
tool contract so it is only invoked when a valid selectionData and session are
present and forbid the model from emitting raw HTML in chat; update the
edit_enhanced_note text to state that the tool must only be called when the
caller provides a specific ProseMirror range via selectionData/session gating,
that the tool should return only the replacement text (plain text or structured
JSON) for that range and never free-form or HTML-wrapped chat output, and
include a short note to validate selectionData/session before performing edits.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
apps/desktop/src/components/editor-area/text-selection-popover.tsx (2)
178-183: Avoid console statements in production.Gate any remaining logs behind
process.env.NODE_ENV !== "production"or a logging utility.- console.warn("DOM selection doesn't match TipTap selection:"); - console.warn("DOM:", selection.text); - console.warn("TipTap:", tipTapPositions.text); + if (process.env.NODE_ENV !== "production") { + console.warn("DOM/Tiptap selection mismatch"); + }
124-165: Derive HTML from ProseMirror slice and scope selection to the editor DOM.Use ProseMirror DOMSerializer to serialize the slice (from/to) to HTML. Also, if a DOM selection exists, ensure it’s within
editor.view.dom. Avoid window-range HTML extraction to prevent mismatches.- // Helper to get TipTap/ProseMirror positions from DOM selection - const getTipTapPositions = () => { + // Why: Ensure HTML is derived from the editor state, not arbitrary DOM. + const getTipTapSelection = () => { const editor = editorRef.current?.editor; if (!editor) { - console.warn("No TipTap editor available"); return null; } - // Get current TipTap selection positions - const { from, to } = editor.state.selection; - - // CLEAN HTML APPROACH: Extract selected content as HTML directly - let selectedHtml = ""; - try { - // Get the selected DOM range - const selection = window.getSelection(); - if (selection && selection.rangeCount > 0) { - const range = selection.getRangeAt(0); - const fragment = range.cloneContents(); - - // Create a temporary div to get the HTML - const tempDiv = document.createElement("div"); - tempDiv.appendChild(fragment); - selectedHtml = tempDiv.innerHTML; - } - - // Fallback: if no DOM selection, use plain text - if (!selectedHtml) { - selectedHtml = editor.state.doc.textBetween(from, to); - } - } catch (error) { - console.warn("Could not extract HTML, falling back to plain text:", error); - selectedHtml = editor.state.doc.textBetween(from, to); - } - - return { - from, - to, - text: selectedHtml, // Now contains HTML instead of plain text - }; + const { state, view } = editor; + const { from, to } = state.selection; + + // If there's a DOM selection, ensure it's inside the editor. + const domSel = window.getSelection(); + if (domSel && domSel.rangeCount > 0) { + const range = domSel.getRangeAt(0); + if (!view.dom.contains(range.commonAncestorContainer)) { + return null; + } + } + + // Serialize slice to HTML + const { DOMSerializer } = require("prosemirror-model"); + const serializer = DOMSerializer.fromSchema(state.schema); + const slice = state.doc.slice(from, to); + const tmp = document.createElement("div"); + tmp.appendChild(serializer.serializeFragment(slice.content)); + const html = tmp.innerHTML; + const text = state.doc.textBetween(from, to); + + return { from, to, html, text }; };Note: add the import (see next comment).
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (3)
86-88: Call previous controls’ cleanup() before removing.Avoid leaked listeners and duplicates.
- // Remove any existing controls - document.querySelector(".ai-edit-controls")?.remove(); + // Remove any existing controls (call cleanup if available) + const prev = document.querySelector(".ai-edit-controls") as any; + if (prev) (prev.cleanup?.() ?? prev.remove());
20-26: Strengthen input schema (ints, bounds, and size).Offsets should be integers with sane bounds; HTML length should be constrained; ensure
endOffset > startOffset.- inputSchema: z.object({ - startOffset: z.number().describe("The ProseMirror start position of the content to replace"), - endOffset: z.number().describe("The ProseMirror end position of the content to replace"), - newHtml: z.string().describe( - "The new HTML content to replace the selected content with. Maintain proper HTML structure and formatting.", - ), - }), + inputSchema: z.object({ + startOffset: z.number().int().nonnegative().describe("ProseMirror start position (inclusive)"), + endOffset: z.number().int().nonnegative().describe("ProseMirror end position (exclusive)"), + newHtml: z.string().min(1).max(20000).describe("Replacement HTML; must be well‑formed."), + }).refine(v => v.endOffset > v.startOffset, { message: "endOffset must be greater than startOffset" }),
45-49: Undo will lose structure/marks; capture original slice, not text.
textBetween(and possiblyselectionData?.text) degrades formatting. Capture the ProseMirror slice and reinsert it on undo.- // Capture original content from selectionData BEFORE making any changes - const originalContent = selectionData?.text || editor.state.doc.textBetween(startOffset, endOffset); + // Capture original content as a ProseMirror slice for lossless undo + const docSize = editor.state.doc.content.size; + const from0 = Math.max(0, Math.min(startOffset, docSize)); + const to0 = Math.max(from0, Math.min(endOffset, docSize)); + const originalNode = editor.state.doc.cut(from0, to0); + const originalJson = (originalNode.toJSON() as any).content ?? [];And in the undo handler:
- .setTextSelection({ from: startOffset, to: highlightEnd }) - .deleteSelection() - .insertContent(originalContent) + .setTextSelection({ from: from0, to: highlightEnd }) + .deleteSelection() + .insertContent(originalJson) .run();apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
220-272: Build the mention node programmatically (avoid HTML string + partial escaping).Concatenating HTML with user-derived labels risks injection; insert a structured node instead.
- const escapedSelectionRef = selectionRef.replace(/"/g, """); - - const referenceText = - `<a class="mention selection-ref" data-mention="true" data-id="selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}" data-type="selection" data-label="${escapedSelectionRef}" contenteditable="false">${selectionRef}</a> `; - - editorRef.current.editor.commands.setContent(referenceText); + const mentionNode = { + type: "mention", + attrs: { + class: "selection-ref", + "data-mention": "true", + "data-id": `selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}`, + "data-type": "selection", + "data-label": selectionRef, + contenteditable: "false", + }, + content: [{ type: "text", text: selectionRef }], + }; + editorRef.current.editor.commands.setContent({ + type: "doc", + content: [{ type: "paragraph", content: [mentionNode, { type: "text", text: " " }] }], + });
🧹 Nitpick comments (6)
apps/desktop/src/components/editor-area/text-selection-popover.tsx (2)
1-8: Add DOMSerializer import required by the serializer usage.import { Button } from "@hypr/ui/components/ui/button"; import { MessageSquare, Sparkles } from "lucide-react"; import { useEffect, useRef, useState } from "react"; +import { DOMSerializer } from "prosemirror-model";
13-16: Remove unusedonAskAIprop fromTextSelectionPopoverPropsand its destructuring inTextSelectionPopover—it isn’t invoked anywhere after the switch tosendSelectionToChat.apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (2)
288-295: Only create tools when actually used (avoid unused work).Creating tools unconditionally does work even when
shouldUseToolsis false. Inline factories in the tools map so they’re only instantiated when needed.- // Create tools using the refactored tool factories - const searchTool = createSearchSessionTool(userId); - const editEnhancedNoteTool = createEditEnhancedNoteTool({ - sessionId, - sessions, - selectionData, - });And below:
tools: { ...(type === "HyprLocal" && { update_progress: tool({ inputSchema: z.any() }) }), - ...(shouldUseTools && { - ...newMcpTools, - search_sessions_multi_keywords: searchTool, - ...hyprMcpTools, - // Add the edit tool when there's selection data - ...(selectionData && { edit_enhanced_note: editEnhancedNoteTool }), - }), + ...(shouldUseTools && { + ...newMcpTools, + search_sessions_multi_keywords: createSearchSessionTool(userId), + ...hyprMcpTools, + ...(selectionData && { + edit_enhanced_note: createEditEnhancedNoteTool({ sessionId, sessions, selectionData }), + }), + }), },
574-575: Nit: don’t pass explicitundefined.Call with three args; avoid the extra
undefined.- await processUserMessage(prompt, "chat_quickaction_sent", undefined, undefined); + await processUserMessage(prompt, "chat_quickaction_sent");apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (2)
107-114: Avoid innerHTML for controls; build DOM nodes.Static strings still trigger security scanners; construct elements safely.
- undoBtn.innerHTML = ` - <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"> - <path d="M3 7v6h6"/><path d="M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13"/> - </svg> - <span style="margin-left: 4px; font-size: 11px;">Undo</span> - `; + { + const svg = document.createElementNS("http://www.w3.org/2000/svg","svg"); + svg.setAttribute("width","12"); svg.setAttribute("height","12"); + svg.setAttribute("viewBox","0 0 24 24"); svg.setAttribute("fill","none"); + svg.setAttribute("stroke","currentColor"); svg.setAttribute("stroke-width","2"); + const p1 = document.createElementNS(svg.namespaceURI,"path"); + p1.setAttribute("d","M3 7v6h6"); + const p2 = document.createElementNS(svg.namespaceURI,"path"); + p2.setAttribute("d","M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13"); + const span = document.createElement("span"); + span.style.marginLeft = "4px"; span.style.fontSize = "11px"; span.textContent = "Undo"; + svg.appendChild(p1); svg.appendChild(p2); undoBtn.appendChild(svg); undoBtn.appendChild(span); + }Similarly for the Accept button (build SVG + span via DOM).
Also applies to: 155-161
37-43: Guideline violation: remove try/catch and console.error (rule: “No error handling”).This file adds error handling that conflicts with the project rules for JS/TS files.
If the rule is intentional, remove the try/catch and logging here or centralize handling upstream. I can provide a focused diff once you confirm policy application in tool executors.
Also applies to: 297-303
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/desktop/src/components/editor-area/index.tsx(3 hunks)apps/desktop/src/components/editor-area/text-selection-popover.tsx(4 hunks)apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(8 hunks)apps/desktop/src/components/right-panel/components/chat/message-content.tsx(0 hunks)apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(6 hunks)apps/desktop/src/components/right-panel/utils/chat-utils.ts(3 hunks)apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts(1 hunks)apps/desktop/src/contexts/ai-edit.tsx(1 hunks)apps/desktop/src/contexts/right-panel.tsx(5 hunks)apps/desktop/src/shared/editor-ref.ts(1 hunks)crates/template/assets/chat.system.jinja(1 hunks)packages/tiptap/src/shared/ai-highlight.ts(1 hunks)packages/tiptap/src/styles/nodes/ai-highlight.css(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/components/right-panel/components/chat/message-content.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/desktop/src/components/editor-area/index.tsx
- apps/desktop/src/contexts/ai-edit.tsx
- apps/desktop/src/shared/editor-ref.ts
- crates/template/assets/chat.system.jinja
- packages/tiptap/src/styles/nodes/ai-highlight.css
- packages/tiptap/src/shared/ai-highlight.ts
- apps/desktop/src/contexts/right-panel.tsx
- apps/desktop/src/components/right-panel/utils/chat-utils.ts
🧰 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/components/chat/chat-input.tsxapps/desktop/src/components/editor-area/text-selection-popover.tsxapps/desktop/src/components/right-panel/hooks/useChatLogic.tsapps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
🧬 Code graph analysis (1)
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (2)
apps/desktop/src/contexts/right-panel.tsx (1)
SelectionData(10-16)apps/desktop/src/shared/editor-ref.ts (1)
globalEditorRef(8-10)
🪛 Biome (2.1.2)
apps/desktop/src/components/editor-area/text-selection-popover.tsx
[error] 32-32: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 ast-grep (0.38.6)
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
[warning] 107-112: 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: undoBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"> <path d="M3 7v6h6"/><path d="M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Undo</span>
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] 155-160: 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: acceptBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="white" stroke-width="2"> <polyline points="20 6 9 17 4 12"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Accept</span>
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] 107-112: 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: undoBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"> <path d="M3 7v6h6"/><path d="M21 17a9 9 0 00-9-9 9 9 0 00-6 2.3L3 13"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Undo</span>
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] 155-160: 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: acceptBtn.innerHTML = <svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="white" stroke-width="2"> <polyline points="20 6 9 17 4 12"/> </svg> <span style="margin-left: 4px; font-size: 11px;">Accept</span>
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-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (4)
apps/desktop/src/components/editor-area/text-selection-popover.tsx (1)
235-251: UI tweaks look good.Button sizing and divider adjustments are consistent with the surrounding UI.
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (2)
301-313: Good: selection context is threaded into history.Passing
selectionDatatoprepareMessageHistoryaligns with the new selection-aware flow.
566-571: Good: submit signature extended to carry selection.This keeps the chat-input and logic in sync.
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
362-401: CSS override set looks good; scoped to selection-ref exclusions.The styling keeps the chat editor lean while preserving selection refs.
| if (!selection) { | ||
| return; | ||
| } | ||
|
|
||
| // Get TipTap/ProseMirror positions (much more accurate) | ||
| const tipTapPositions = getTipTapPositions(); | ||
| if (!tipTapPositions) { | ||
| console.error("Could not get TipTap positions"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Send both HTML and plain text; gate debug logs; use the new helper.
Use getTipTapSelection(), include html alongside text, and avoid console noise in production.
- const handleAskAIClick = () => {
+ const handleAskAIClick = () => {
if (!selection) {
return;
}
- // Get TipTap/ProseMirror positions (much more accurate)
- const tipTapPositions = getTipTapPositions();
- if (!tipTapPositions) {
- console.error("Could not get TipTap positions");
- return;
- }
+ const tipTap = getTipTapSelection();
+ if (!tipTap) return;
- // Verify DOM selection matches TipTap selection
- if (selection.text.trim() !== tipTapPositions.text.trim()) {
- console.warn("DOM selection doesn't match TipTap selection:");
- console.warn("DOM:", selection.text);
- console.warn("TipTap:", tipTapPositions.text);
- }
+ if (process.env.NODE_ENV !== "production") {
+ const plain = tipTap.text?.trim() ?? "";
+ if (selection.text.trim() !== plain) {
+ // Dev-only mismatch signal
+ console.warn("Selection mismatch between DOM and TipTap slice.");
+ }
+ }
- const selectionData = {
- text: tipTapPositions.text, // Use TipTap's text (more reliable)
- startOffset: tipTapPositions.from, // ProseMirror position
- endOffset: tipTapPositions.to, // ProseMirror position
- sessionId,
- timestamp: Date.now(),
- };
+ const selectionData = {
+ text: tipTap.text, // plain text for prompts
+ html: tipTap.html, // HTML for precise edits/highlights
+ startOffset: tipTap.from, // ProseMirror pos
+ endOffset: tipTap.to, // ProseMirror pos
+ sessionId,
+ timestamp: Date.now(),
+ };
// Send selection to chat
sendSelectionToChat(selectionData);Also applies to: 178-183, 185-195
🤖 Prompt for AI Agents
In apps/desktop/src/components/editor-area/text-selection-popover.tsx around
lines 167-176 (and similarly for 178-183 and 185-195), the code uses
getTipTapPositions(), logs directly to console, and only sends plain text;
replace calls to getTipTapPositions() with the new getTipTapSelection() helper,
include both html and text properties when sending the selection, and wrap debug
logs behind a development check or use the app logger instead of console.error
to avoid production noise; ensure the function returns early if
getTipTapSelection() yields no data and propagate both html and text to
downstream callers.
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
Show resolved
Hide resolved
apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 10
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/message-content.tsx (1)
75-91: Safe fallback on markdown→HTML failureOn conversion failure you set displayHtml to raw content, which the Renderer will treat as HTML. Escape it or sanitize to avoid injection.
try { let html = await miscCommands.opinionatedMdToHtml(content); 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); + setDisplayHtml(DOMPurify.sanitize(html)); } catch (error) { - console.error("Failed to convert markdown:", error); - setDisplayHtml(content); + // Fallback: render as plain text safely + const escaped = content + .replaceAll("&", "&") + .replaceAll("<", "<") + .replaceAll(">", ">"); + setDisplayHtml(`<pre style="white-space:pre-wrap;margin:0">${escaped}</pre>`); }
♻️ Duplicate comments (5)
crates/template/assets/chat.system.jinja (1)
163-167: Harden edit_enhanced_note contract; forbid raw-HTML replies; require selectionData/session gating.Tighten the tool guidance so the model only edits via the tool when valid selectionData is present, and never emits raw HTML in normal chat. Also clarify the date-range tool’s contract.
- - edit_enhanced_note: Only call when you are given a certain Prosemirror range of text that the user wants to edit. Replace that certain range with a new text that is written in a way - that the user wants. + - edit_enhanced_note: Call only when selectionData is present (must include startOffset, endOffset, sessionId) and the active editor is available. Validate selectionData/session before editing. Provide an HTML fragment that replaces exactly that ProseMirror range, preserving nodes/marks and not altering content outside the range. Do NOT output HTML in normal chat responses—perform edits exclusively via this tool. - - search_sessions_date_range: Search for sessions (meeting notes) within a specific date range. Returns sessions with their enhanced memo content from the specified time period. + - search_sessions_date_range: Search for sessions (meeting notes) within a specific date range. Input: ISO 8601 startDate and endDate with timezone (e.g., 2024-01-01T00:00:00Z), and an optional limit (default 50). Returns sessions in that period with their memo content; summarize concisely (no raw dumps in ```).apps/desktop/src/components/editor-area/text-selection-popover.tsx (2)
29-43: Fix conditional hook usage (Rules of Hooks violation).useRightPanel() is invoked inside an IIFE/try-catch. Call hooks at top level.
- // Safe hook usage with fallback - const rightPanel = (() => { - try { - return useRightPanel(); - } catch { - return { - sendSelectionToChat: () => { - console.warn("RightPanel not available - selection ignored"); - }, - }; - } - })(); - - const { sendSelectionToChat } = rightPanel; + // RightPanel provider is required; call hook at top level. + const { sendSelectionToChat } = useRightPanel();
173-204: Use the new helper; send both text and html; dev-only mismatch logs.Prevents noisy consoles and ambiguous payloads.
const handleAskAIClick = () => { if (!selection) { return; } - // Get TipTap/ProseMirror positions (much more accurate) - const tipTapPositions = getTipTapPositions(); - if (!tipTapPositions) { - console.error("Could not get TipTap positions"); - return; - } + const tipTap = getTipTapSelection(); + if (!tipTap) return; - // Verify DOM selection matches TipTap selection - if (selection.text.trim() !== tipTapPositions.text.trim()) { - console.warn("DOM selection doesn't match TipTap selection:"); - console.warn("DOM:", selection.text); - console.warn("TipTap:", tipTapPositions.text); - } + if (process.env.NODE_ENV !== "production") { + if (selection.text.trim() !== tipTap.text.trim()) { + console.warn("Selection mismatch between DOM and TipTap slice."); + } + } const selectionData = { - text: tipTapPositions.text, // Use TipTap's text (more reliable) - startOffset: tipTapPositions.from, // ProseMirror position - endOffset: tipTapPositions.to, // ProseMirror position + text: tipTap.text, + html: tipTap.html, + startOffset: tipTap.from, + endOffset: tipTap.to, sessionId, timestamp: Date.now(), }; - // Send selection to chat sendSelectionToChat(selectionData); setSelection(null); };apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (1)
460-461: Remove debug log of tool-result chunk (PII risk + noise)This duplicates a prior review; please drop it.
- console.log("tool result: ", chunk); + // removed debug logapps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
227-280: Build the mention/reference node programmatically (avoid HTML string concat)Current HTML string assembly is vulnerable to injection via selectionRef inner text. Use TipTap JSON with escaped text.
- const referenceText = - `<a class="mention selection-ref" data-mention="true" data-id="selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}" data-type="selection" data-label="${escapedSelectionRef}" contenteditable="false">${selectionRef}</a> `; - - editorRef.current.editor.commands.setContent(referenceText); - editorRef.current.editor.commands.focus("end"); + const escapeText = (s: string) => + s.replaceAll("&", "&").replaceAll("<", "<").replaceAll(">", ">"); + const mentionNode = { + type: "mention", + attrs: { + class: "mention selection-ref", + "data-mention": "true", + id: `selection-${pendingSelection.startOffset}-${pendingSelection.endOffset}`, + type: "selection", + label: escapedSelectionRef, + contenteditable: "false", + }, + content: [{ type: "text", text: escapeText(selectionRef) }], + }; + editorRef.current.editor.commands.clearContent(); + editorRef.current.editor.commands.insertContent(mentionNode); + editorRef.current.editor.commands.focus("end");If your mention extension requires specific attrs (id/type/label), align key names accordingly. The point is to avoid innerHTML.
🧹 Nitpick comments (12)
packages/tiptap/src/renderer/mention-readonly.ts (2)
54-90: Trim parsed labels to prevent stray whitespace.Ensure consistent rendering when labels come from textContent.
Apply:
- const label = dom.getAttribute("data-label") || dom.textContent || ""; + const label = ((dom.getAttribute("data-label") ?? dom.textContent) ?? "").trim();Repeat the same change in the other two getAttrs blocks.
115-118: Mirror the empty-label guard in renderText.Apply:
- const label = node.attrs.label || ""; - return label.startsWith("@") ? label : `@${label}`; + const label = (node.attrs.label || "").trim(); + return label ? (label.startsWith("@") ? label : `@${label}`) : "";apps/desktop/src/components/right-panel/utils/tools/search_session_date_range.ts (2)
20-31: Pre-validate chronological order at schema or coerce dates once.Avoid double parsing and centralize validation.
- execute: async ({ startDate, endDate, limit }) => { - // Validate date format - const start = new Date(startDate); - const end = new Date(endDate); + execute: async ({ startDate, endDate, limit }) => { + const start = new Date(startDate); + const end = new Date(endDate); if (isNaN(start.getTime()) || isNaN(end.getTime())) { throw new Error("Invalid date format. Please use ISO 8601 format (e.g., '2024-01-01T00:00:00Z')"); } if (start >= end) { throw new Error("Start date must be before end date"); }(Optional follow-up: lift the chronological check into a zod .refine to fail earlier.)
65-79: Avoid repeated O(n) counts; compute once.Minor perf cleanup for larger result sets.
- return { - sessions: nonEmptySessions, - metadata: { - total_found: sessions.length, - non_empty_sessions: nonEmptySessions.length, - date_range: { - start: startDate, - end: endDate, - }, - query_limit: limit, - sessions_with_enhanced_content: nonEmptySessions.filter(s => s.has_enhanced_content).length, - sessions_with_raw_content: nonEmptySessions.filter(s => s.has_raw_content).length, - sessions_with_pre_meeting_content: nonEmptySessions.filter(s => s.has_pre_meeting_content).length, - }, - }; + const counts = nonEmptySessions.reduce( + (acc, s) => { + if (s.has_enhanced_content) acc.enhanced++; + if (s.has_raw_content) acc.raw++; + if (s.has_pre_meeting_content) acc.pre++; + return acc; + }, + { enhanced: 0, raw: 0, pre: 0 }, + ); + return { + sessions: nonEmptySessions, + metadata: { + total_found: sessions.length, + non_empty_sessions: nonEmptySessions.length, + date_range: { start: startDate, end: endDate }, + query_limit: limit, + sessions_with_enhanced_content: counts.enhanced, + sessions_with_raw_content: counts.raw, + sessions_with_pre_meeting_content: counts.pre, + }, + };apps/desktop/src/components/editor-area/text-selection-popover.tsx (1)
13-16: Remove unused onAskAI prop.Prop is not used; keep API surface minimal.
interface TextSelectionPopoverProps { isEnhancedNote: boolean; onAnnotate: (selectedText: string, selectedRect: DOMRect) => void; - onAskAI?: (selectedText: string) => void; isAnnotationBoxOpen: boolean; sessionId: string; editorRef: React.RefObject<{ editor: any }>; } @@ -export function TextSelectionPopover( - { isEnhancedNote, onAnnotate, onAskAI, isAnnotationBoxOpen, sessionId, editorRef }: TextSelectionPopoverProps, -) { +export function TextSelectionPopover( + { isEnhancedNote, onAnnotate, isAnnotationBoxOpen, sessionId, editorRef }: TextSelectionPopoverProps, +) {Also applies to: 24-25
apps/desktop/src/components/right-panel/hooks/useChatQueries.ts (2)
82-102: Don’t log parse errors in production; avoid any.Gate logs or use app logger, and avoid
any.- return dbMessages.map(msg => { + return dbMessages.map(msg => { // Parse tool_details for all messages - let parsedToolDetails: any = undefined; + let parsedToolDetails: unknown = undefined; if (msg.tool_details) { try { parsedToolDetails = JSON.parse(msg.tool_details); } catch (error) { - console.error("Failed to parse tool_details:", msg.id, error); + if (process.env.NODE_ENV !== "production") { + console.warn("Failed to parse tool_details:", msg.id, error); + } } } return {
119-120: Avoid noisy sync logs while generating.Gate behind dev check.
- console.log("Skipping DB sync - currently generating"); + if (process.env.NODE_ENV !== "production") { + console.log("Skipping DB sync - currently generating"); + }apps/desktop/src/components/right-panel/components/chat/message-content.tsx (2)
94-98: Avoid unnecessary work when both inputs are emptyGuard also against both content/htmlContent being empty strings to prevent spawning processContent.
- }, [content, htmlContent]); + }, [content, htmlContent]);Optional: early-return if neither has length after trim (micro-opt; no behavior change).
171-190: CSS mention styles: scope to component to avoid leakageThe selectors are broad and use !important everywhere. Consider scoping with a unique wrapper class to reduce style collisions.
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (3)
281-309: Keydown handler: narrow mention dropdown detectionGlobal querySelector can pick up unrelated elements; if possible, scope to editor’s root or a known container.
- const mentionDropdown = document.querySelector(".mention-container"); + const root = editor?.view.dom?.ownerDocument || document; + const mentionDropdown = root.querySelector(".mention-container");
365-447: CSS scope nitThe heavy use of !important under .chat-editor could leak. Consider a wrapper class (e.g., .chat-input-root) to tighten specificity instead of !important.
219-220: Comments should focus on “Why,” not “What”Several comments narrate the code (“Extract HTML content…”, “Clear the selection…”). Trim or rephrase to rationale per coding guideline.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
apps/desktop/src/components/editor-area/text-selection-popover.tsx(5 hunks)apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(8 hunks)apps/desktop/src/components/right-panel/components/chat/message-content.tsx(4 hunks)apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(9 hunks)apps/desktop/src/components/right-panel/hooks/useChatQueries.ts(1 hunks)apps/desktop/src/components/right-panel/utils/tools/search_session_date_range.ts(1 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(1 hunks)crates/template/assets/chat.system.jinja(1 hunks)packages/tiptap/src/renderer/index.tsx(2 hunks)packages/tiptap/src/renderer/mention-readonly.ts(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/utils/tools/search_session_date_range.tsapps/desktop/src/components/right-panel/hooks/useChatQueries.tspackages/tiptap/src/renderer/index.tsxpackages/tiptap/src/renderer/mention-readonly.tsapps/desktop/src/components/editor-area/text-selection-popover.tsxapps/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/message-content.tsxapps/desktop/src/components/right-panel/hooks/useChatLogic.ts
🧬 Code graph analysis (5)
apps/desktop/src/components/right-panel/hooks/useChatQueries.ts (1)
apps/desktop/src/components/right-panel/utils/markdown-parser.ts (1)
parseMarkdownBlocks(3-62)
packages/tiptap/src/renderer/index.tsx (1)
packages/tiptap/src/renderer/mention-readonly.ts (1)
mentionReadonly(8-119)
apps/desktop/src/components/editor-area/text-selection-popover.tsx (3)
apps/desktop/src/contexts/hypr.tsx (1)
useHypr(63-69)apps/desktop/src/contexts/right-panel.tsx (1)
useRightPanel(230-236)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (1)
apps/desktop/src/contexts/right-panel.tsx (2)
SelectionData(10-16)useRightPanel(230-236)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (5)
apps/desktop/src/contexts/right-panel.tsx (1)
SelectionData(10-16)apps/desktop/src/components/right-panel/components/chat/types.ts (1)
Message(7-15)apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts (1)
createSearchSessionTool(5-59)apps/desktop/src/components/right-panel/utils/tools/edit_enhanced_note.ts (1)
createEditEnhancedNoteTool(12-306)apps/desktop/src/components/right-panel/utils/tools/search_session_date_range.ts (1)
createSearchSessionDateRangeTool(5-82)
🪛 Biome (2.1.2)
apps/desktop/src/components/editor-area/text-selection-popover.tsx
[error] 32-32: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ 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-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (14)
packages/tiptap/src/renderer/mention-readonly.ts (1)
21-51: Attributes setup looks good.packages/tiptap/src/renderer/index.tsx (1)
2-2: CSS import for mentions is appropriate.apps/desktop/src/components/editor-area/text-selection-popover.tsx (1)
70-74: Good: scope selection to editor DOM.Ensures we only act on selections within the editor.
Also applies to: 76-76
apps/desktop/src/components/right-panel/views/chat-view.tsx (1)
183-184: Confirm tri-arg onSubmit contract end-to-end.Ensure ChatInput’s prop type and useChatLogic.handleSubmit both accept (mentionedContent, selectionData, htmlContent) in this order.
apps/desktop/src/components/right-panel/components/chat/message-content.tsx (2)
336-346: Pass htmlContent only when sanitizedYou’re passing htmlContent through to MarkdownText. Ensure it’s sanitized upstream (see earlier comment) or pass a sanitizedHtml variable instead.
Would you like me to wire a sanitizedHtml into MessageContent to make the contract explicit?
194-195: Verify Renderer input type
Renderer is imported from@hypr/tiptap/renderer; confirm whether itsinitialContentprop expects sanitized HTML or TipTap JSON. If it requires JSON, convertdisplayHtmlwith the editor’s parser; otherwise keep the HTML sanitization.apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (5)
297-305: Tool factory wiring LGTMFactory extraction clarifies responsibilities and enables conditional tool exposure.
320-321: Selection context propagation LGTMPassing selectionData into prepareMessageHistory is necessary for tool prompting.
325-333: Tool map composition: order and gating look correctInclusion of edit_enhanced_note behind selectionData is correct. No duplicate keys detected.
578-584: No action needed: ChatInput callers already updated
The only<ChatInput>instance in chat-view.tsx passes(mentionedContent, selectionData, htmlContent)tohandleSubmitin the correct order, so all callers reflect the new signature.
121-127: Call sites updated correctly
Verified that allprocessUserMessagecalls include the newselectionDataandhtmlContentparameters in the proper order (or fall back to defaults), and allhandleSubmitinvocations either pass all three args in order or omit them to use undefined defaults.apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (3)
144-145: Dedup key is goodprocessedSelectionRef prevents duplicate insertions across rerenders. Nice.
47-48: Context usage LGTMUsing pendingSelection/clearPendingSelection from RightPanel context aligns with the new flow.
191-201: Event metric handling LGTMMention extraction and analytics event cadence unaffected by this change.
| // Helper to get TipTap/ProseMirror positions from DOM selection | ||
| const getTipTapPositions = () => { | ||
| const editor = editorRef.current?.editor; | ||
| if (!editor) { | ||
| console.warn("No TipTap editor available"); | ||
| return null; | ||
| } | ||
|
|
||
| // Get current TipTap selection positions | ||
| const { from, to } = editor.state.selection; | ||
|
|
||
| // CLEAN HTML APPROACH: Extract selected content as HTML directly | ||
| let selectedHtml = ""; | ||
| try { | ||
| // Get the selected DOM range | ||
| const selection = window.getSelection(); | ||
| if (selection && selection.rangeCount > 0) { | ||
| const range = selection.getRangeAt(0); | ||
| const fragment = range.cloneContents(); | ||
|
|
||
| // Create a temporary div to get the HTML | ||
| const tempDiv = document.createElement("div"); | ||
| tempDiv.appendChild(fragment); | ||
| selectedHtml = tempDiv.innerHTML; | ||
| } | ||
|
|
||
| // Fallback: if no DOM selection, use plain text | ||
| if (!selectedHtml) { | ||
| selectedHtml = editor.state.doc.textBetween(from, to); | ||
| } | ||
| } catch (error) { | ||
| console.warn("Could not extract HTML, falling back to plain text:", error); | ||
| selectedHtml = editor.state.doc.textBetween(from, to); | ||
| } | ||
|
|
||
| return { | ||
| from, | ||
| to, | ||
| text: selectedHtml, // Now contains HTML instead of plain text | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return both plain text and HTML; gate dev logs; clear naming.
Current helper names the HTML as “text” and logs to console unconditionally. Provide text and html separately, and avoid prod logging.
- // Helper to get TipTap/ProseMirror positions from DOM selection
- const getTipTapPositions = () => {
+ // Helper: derive ProseMirror range + text/html slice from current selection
+ const getTipTapSelection = () => {
const editor = editorRef.current?.editor;
if (!editor) {
- console.warn("No TipTap editor available");
+ if (process.env.NODE_ENV !== "production") {
+ console.warn("No TipTap editor available");
+ }
return null;
}
- // Get current TipTap selection positions
const { from, to } = editor.state.selection;
- // CLEAN HTML APPROACH: Extract selected content as HTML directly
- let selectedHtml = "";
+ let html = "";
+ const text = editor.state.doc.textBetween(from, to);
try {
- // Get the selected DOM range
const selection = window.getSelection();
if (selection && selection.rangeCount > 0) {
const range = selection.getRangeAt(0);
const fragment = range.cloneContents();
- // Create a temporary div to get the HTML
const tempDiv = document.createElement("div");
tempDiv.appendChild(fragment);
- selectedHtml = tempDiv.innerHTML;
+ html = tempDiv.innerHTML;
}
- // Fallback: if no DOM selection, use plain text
- if (!selectedHtml) {
- selectedHtml = editor.state.doc.textBetween(from, to);
- }
+ if (!html) html = text;
} catch (error) {
- console.warn("Could not extract HTML, falling back to plain text:", error);
- selectedHtml = editor.state.doc.textBetween(from, to);
+ if (process.env.NODE_ENV !== "production") {
+ console.warn("HTML extract failed; falling back to text.", error);
+ }
+ html = text;
}
return {
from,
to,
- text: selectedHtml, // Now contains HTML instead of plain text
+ text,
+ html,
};
};📝 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.
| // Helper to get TipTap/ProseMirror positions from DOM selection | |
| const getTipTapPositions = () => { | |
| const editor = editorRef.current?.editor; | |
| if (!editor) { | |
| console.warn("No TipTap editor available"); | |
| return null; | |
| } | |
| // Get current TipTap selection positions | |
| const { from, to } = editor.state.selection; | |
| // CLEAN HTML APPROACH: Extract selected content as HTML directly | |
| let selectedHtml = ""; | |
| try { | |
| // Get the selected DOM range | |
| const selection = window.getSelection(); | |
| if (selection && selection.rangeCount > 0) { | |
| const range = selection.getRangeAt(0); | |
| const fragment = range.cloneContents(); | |
| // Create a temporary div to get the HTML | |
| const tempDiv = document.createElement("div"); | |
| tempDiv.appendChild(fragment); | |
| selectedHtml = tempDiv.innerHTML; | |
| } | |
| // Fallback: if no DOM selection, use plain text | |
| if (!selectedHtml) { | |
| selectedHtml = editor.state.doc.textBetween(from, to); | |
| } | |
| } catch (error) { | |
| console.warn("Could not extract HTML, falling back to plain text:", error); | |
| selectedHtml = editor.state.doc.textBetween(from, to); | |
| } | |
| return { | |
| from, | |
| to, | |
| text: selectedHtml, // Now contains HTML instead of plain text | |
| }; | |
| }; | |
| // Helper: derive ProseMirror range + text/html slice from current selection | |
| const getTipTapSelection = () => { | |
| const editor = editorRef.current?.editor; | |
| if (!editor) { | |
| if (process.env.NODE_ENV !== "production") { | |
| console.warn("No TipTap editor available"); | |
| } | |
| return null; | |
| } | |
| const { from, to } = editor.state.selection; | |
| let html = ""; | |
| const text = editor.state.doc.textBetween(from, to); | |
| try { | |
| const selection = window.getSelection(); | |
| if (selection && selection.rangeCount > 0) { | |
| const range = selection.getRangeAt(0); | |
| const fragment = range.cloneContents(); | |
| const tempDiv = document.createElement("div"); | |
| tempDiv.appendChild(fragment); | |
| html = tempDiv.innerHTML; | |
| } | |
| if (!html) html = text; | |
| } catch (error) { | |
| if (process.env.NODE_ENV !== "production") { | |
| console.warn("HTML extract failed; falling back to text.", error); | |
| } | |
| html = text; | |
| } | |
| return { | |
| from, | |
| to, | |
| text, | |
| html, | |
| }; | |
| }; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/editor-area/text-selection-popover.tsx around
lines 131 to 171, the helper currently stores HTML in a variable named
"selectedHtml" but returns it as "text" and unconditionally uses console.warn;
update the function to produce and return two explicit fields (plainText and
html), rename variables for clarity (e.g., selectedHtml and selectedText),
ensure plainText is derived via editor.state.doc.textBetween(from, to) as
fallback or alongside HTML extraction, and gate any warning logs behind a
dev-only check (e.g., if (process.env.NODE_ENV !== 'production') or use the app
logger) so production builds do not emit console warnings. Ensure the returned
object is { from, to, plainText, html } and adjust call sites accordingly.
apps/desktop/src/components/right-panel/components/chat/message-content.tsx
Show resolved
Hide resolved
apps/desktop/src/components/right-panel/components/chat/message-content.tsx
Show resolved
Hide resolved
apps/desktop/src/components/right-panel/utils/tools/search_session_date_range.ts
Show resolved
Hide resolved
apps/desktop/src/components/right-panel/utils/tools/search_session_date_range.ts
Show resolved
Hide resolved
| name: "mention", | ||
|
|
||
| group: "inline", | ||
|
|
||
| inline: true, | ||
|
|
||
| selectable: false, | ||
|
|
||
| atom: true, | ||
|
|
||
| addAttributes() { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid potential extension name collision ("mention").
If any extension named "mention" is already included via shared.extensions, this will collide. Ensure the Renderer excludes any existing "mention" before adding this one.
Run to confirm duplicates:
🏁 Script executed:
#!/bin/bash
# Find any extensions declaring name: "mention"
rg -nP -C2 'Node\.create\(\s*{\s*name:\s*["\']mention["\']' --type=ts
# Find imports of @tiptap/extension-mention
rg -nP '@tiptap/extension-mention' --type=tsLength of output: 116
🏁 Script executed:
#!/bin/bash
# search for other occurrences of name: "mention" in the codebase
rg -nP "name\\s*:\\s*['\\\"]mention['\\\"]" --type=ts -C3
# search for usage/import of @tiptap/extension-mention
rg -nP "@tiptap/extension-mention" --type=ts -C3Length of output: 1150
Exclude existing “mention” extensions before registering the readonly version
Filter out any extensions with name === "mention" (e.g. from @tiptap/extension-mention) in your renderer’s shared.extensions array before adding mentionReadonly to prevent duplicate‐name collisions.
🤖 Prompt for AI Agents
In packages/tiptap/src/renderer/mention-readonly.ts around lines 9 to 19, avoid
duplicate extension names by removing any existing extensions whose name ===
"mention" from shared.extensions before adding the readonly mention extension;
implement this by creating a filtered array (e.g. const filtered =
shared.extensions.filter(e => e.name !== 'mention')) and then register or spread
filtered plus mentionReadonly (or assign shared.extensions =
filtered.concat(mentionReadonly)) so the readonly extension is the only
"mention" entry.
| renderHTML({ node, HTMLAttributes }) { | ||
| const label = node.attrs.label || ""; | ||
| const nodeText = node.textContent || label; | ||
| const classes = ["mention"]; | ||
|
|
||
| // Add selection-ref class if this is a selection reference | ||
| if (label.includes("[") && label.includes("]")) { | ||
| classes.push("selection-ref"); | ||
| } | ||
|
|
||
| return [ | ||
| "span", | ||
| mergeAttributes(HTMLAttributes, { | ||
| class: classes.join(" "), | ||
| "data-mention": "true", | ||
| "data-id": node.attrs.id, | ||
| "data-type": node.attrs.type, | ||
| "data-label": node.attrs.label, | ||
| }), | ||
| nodeText.startsWith("@") ? nodeText : `@${nodeText}`, | ||
| ]; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t re-specify data-id/type/label here; rely on addAttributes to avoid null/undefined attributes.
Also guard against empty text to avoid emitting a lone "@".
Apply:
renderHTML({ node, HTMLAttributes }) {
const label = node.attrs.label || "";
const nodeText = node.textContent || label;
const classes = ["mention"];
// Add selection-ref class if this is a selection reference
if (label.includes("[") && label.includes("]")) {
classes.push("selection-ref");
}
- return [
- "span",
- mergeAttributes(HTMLAttributes, {
- class: classes.join(" "),
- "data-mention": "true",
- "data-id": node.attrs.id,
- "data-type": node.attrs.type,
- "data-label": node.attrs.label,
- }),
- nodeText.startsWith("@") ? nodeText : `@${nodeText}`,
- ];
+ const displayText =
+ nodeText ? (nodeText.startsWith("@") ? nodeText : `@${nodeText}`) : "";
+ return [
+ "span",
+ mergeAttributes(HTMLAttributes, {
+ class: classes.join(" "),
+ "data-mention": "true",
+ }),
+ displayText,
+ ];
},📝 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.
| renderHTML({ node, HTMLAttributes }) { | |
| const label = node.attrs.label || ""; | |
| const nodeText = node.textContent || label; | |
| const classes = ["mention"]; | |
| // Add selection-ref class if this is a selection reference | |
| if (label.includes("[") && label.includes("]")) { | |
| classes.push("selection-ref"); | |
| } | |
| return [ | |
| "span", | |
| mergeAttributes(HTMLAttributes, { | |
| class: classes.join(" "), | |
| "data-mention": "true", | |
| "data-id": node.attrs.id, | |
| "data-type": node.attrs.type, | |
| "data-label": node.attrs.label, | |
| }), | |
| nodeText.startsWith("@") ? nodeText : `@${nodeText}`, | |
| ]; | |
| }, | |
| renderHTML({ node, HTMLAttributes }) { | |
| const label = node.attrs.label || ""; | |
| const nodeText = node.textContent || label; | |
| const classes = ["mention"]; | |
| // Add selection-ref class if this is a selection reference | |
| if (label.includes("[") && label.includes("]")) { | |
| classes.push("selection-ref"); | |
| } | |
| const displayText = nodeText | |
| ? (nodeText.startsWith("@") ? nodeText : `@${nodeText}`) | |
| : ""; | |
| return [ | |
| "span", | |
| mergeAttributes(HTMLAttributes, { | |
| class: classes.join(" "), | |
| "data-mention": "true", | |
| }), | |
| displayText, | |
| ]; | |
| }, |
🤖 Prompt for AI Agents
In packages/tiptap/src/renderer/mention-readonly.ts around lines 92 to 113, the
renderHTML implementation is re-specifying data-id/data-type/data-label (which
can produce null/undefined) instead of relying on addAttributes and it can emit
a lone "@" when node text is empty; remove the explicit
"data-id"/"data-type"/"data-label" keys from the merged attributes and only
merge HTMLAttributes (so addAttributes provides those values), and before
returning the displayed text compute nodeText safely (use label fallback) and if
nodeText is empty/whitespace return an empty string instead of "@", otherwise
prefix with "@" only when not already present.
1 participant
DON'T MERGE THIS YET
Summary by cubic
Select text in an enhanced note, send it to chat, and apply edits to that exact range. Edits insert HTML, get highlighted inline, and include accept/undo controls.
New Features
Refactors