Conversation
|
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 MCP tooling and UI: new settings page and hook for MCP servers, dynamic tool wrappers, streaming overhaul that emits typed chat events and persists partial/tool messages, DB migration and types for message Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ChatView / ChatMessagesView
participant Hook as useChatLogic
participant MCP as useMcpTools / MCP Clients
participant LLM as LLM (fullStream)
participant DB as DB (chat_messages)
UI->>Hook: submit user message
Hook->>DB: persist user message
Hook->>MCP: fetch enabled servers/tools
MCP-->>Hook: dynamicTool[] (wrapped)
Hook->>LLM: start fullStream (history + tools)
activate LLM
loop streaming
LLM-->>Hook: chunk (text-delta | tool-start | tool-result | tool-error)
alt text-delta
Hook->>DB: upsert text-delta segment
Hook-->>UI: update current AI message
else tool-*
Hook->>DB: insert tool-* message
Hook-->>UI: append tool card
end
end
LLM-->>Hook: finish
deactivate LLM
Hook->>MCP: close clients
Hook-->>UI: set isGenerating=false, isStreamingText=false
sequenceDiagram
participant Hook as useChatLogic
participant UI as ChatMessagesView
participant State as Local debounce state
Hook-->>UI: isGenerating / isStreamingText
UI->>State: evaluate shouldShowThinking
State-->>State: debounce 200ms
State-->>UI: showThinking=true
UI->>UI: render ThinkingIndicator and scroll
Hook-->>UI: streaming text arrives
UI->>State: showThinking=false (re-render)
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. ✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
crates/db-user/src/chat_messages_migration_1.sql (1)
1-5: Maketypecolumn NOT NULL with DEFAULTSQLite (and Postgres) will populate existing rows with a constant DEFAULT, so an extra
UPDATEisn’t needed. To align the schema with your non-Option Rust enum, mark the new column NOT NULL:• File:
crates/db-user/src/chat_messages_migration_1.sqlALTER TABLE chat_messages -ADD COLUMN type TEXT DEFAULT 'text-delta'; +ADD COLUMN type TEXT NOT NULL DEFAULT 'text-delta';
♻️ Duplicate comments (1)
apps/desktop/src/components/settings/components/types.ts (1)
37-37: Single source of truth for icons to avoid divergence with TabIconThere is historical drift between TABS icons and TabIcon’s switch for some tabs. Suggest consuming TABS in TabIcon (see TabIcon comment) to prevent future inconsistencies.
🧹 Nitpick comments (29)
apps/desktop/src/components/editor-area/index.tsx (1)
480-482: Gate debug logs and avoid dumping full provider objectThe raw
console.logcalls at lines 480–482 will ship to production and can expose internal details. Replace them with gated debug statements in development builds:--- a/apps/desktop/src/components/editor-area/index.tsx +++ b/apps/desktop/src/components/editor-area/index.tsx @@ -478,6 +478,11 @@ : provider.languageModel("defaultModel"); - console.log("model: ", model); - console.log("provider: ", provider); + if (import.meta.env.DEV) { + console.debug("[enhance] model:", model); + console.debug( + "[enhance] provider:", + (provider as any)?.constructor?.name ?? "unknown" + ); + } if (sessionId !== onboardingSessionId) {• File to update:
– apps/desktop/src/components/editor-area/index.tsx (lines 480–482)Optionally, scan for any remaining unguarded
console.*calls before merging:#!/bin/bash rg -n -C2 'console\.(log|debug|info|warn|error)\(' -g '*.ts' -g '*.tsx'apps/desktop/src/utils/broadcast.ts (1)
78-82: MCP-tools cache invalidation verifiedAll occurrences of
"mcp-tools"in your query keys have been located and match the invalidation logic:
- apps/desktop/src/utils/broadcast.ts (lines 78–82) – invalidates on
keys[0] === "mcp-tools".- apps/desktop/src/components/right-panel/hooks/useMcpTools.ts (line 12) – issues
useQuery({ queryKey: ["mcp-tools"], … }).This ensures the hook’s cache is kept in sync across windows.
Optional future-proofing: switch to a predicate to catch any parameterized variants (e.g.
["mcp-tools", id]):- if (keys[0] === "mcp-tools") { - queryClient.invalidateQueries({ - queryKey: ["mcp-tools"], - }); - } + if (keys[0] === "mcp-tools") { + queryClient.invalidateQueries({ + predicate: (q) => + Array.isArray(q.queryKey) && + q.queryKey[0] === "mcp-tools", + }); + }crates/template/assets/enhance.system.jinja (2)
38-39: Strengthened no–chain-of-thought directive: consider tightening for consistencyGood addition. To avoid mixed signals with later “Think step by step”, consider removing or rephrasing that line to reduce the chance of models emitting reasoning.
-CRITICAL: Do not include any of your thought process or comments. Just return the end result of the enhanced note, do not explain or justify your results (regardless of whether template is given or not) +CRITICAL: Do not include any thought process or comments. Return only the final enhanced note; do not explain or justify results (regardless of whether a template is provided).Outside this hunk, consider updating the guidance at Line 52:
-You will be given multiple inputs from the user. Below are useful information that you will use to write the best enhanced meeting note. Think step by step. +You will be given multiple inputs from the user. Use them carefully to produce the best enhanced meeting note. Plan internally; do not include your reasoning in the output.
85-86: Duplicate/no‑CoT instruction: consistent and clear, minor wording tweakTo keep tone consistent with the earlier CRITICAL block and avoid accidental CoT leakage, small clarity pass suggested.
-- Go straight to the actual note part, never include any of your thought process or comments. Just return the meeting note, do not explain or justify your results (regardless of whether template is given or not) +- Go straight to the note content. Never include thought process or comments. Return only the meeting note; do not explain or justify results (regardless of whether a template is provided).apps/desktop/src/locales/ko/messages.po (1)
1029-1032: Avoid empty label: provide a Korean translation (or fallback) for “MCP”.Empty msgstr can render as a blank tab label depending on i18n fallback behavior. Safe choice is to mirror English here.
Apply this diff:
-#: src/routes/app.settings.tsx:73 -msgid "MCP" -msgstr "" +#: src/routes/app.settings.tsx:73 +msgid "MCP" +msgstr "MCP"crates/db-user/src/chat_messages_types.rs (1)
12-26: Reduce duplication with enum-level rename directives.Use enum-level kebab-case for serde and strum to remove per-variant attributes.
Apply this diff:
user_common_derives! { - #[derive(strum::EnumString, strum::Display)] + #[derive(strum::EnumString, strum::Display)] + #[serde(rename_all = "kebab-case")] + #[strum(serialize_all = "kebab-case")] pub enum ChatMessageType { - #[serde(rename = "text-delta")] - #[strum(serialize = "text-delta")] TextDelta, - #[serde(rename = "tool-start")] - #[strum(serialize = "tool-start")] ToolStart, - #[serde(rename = "tool-result")] - #[strum(serialize = "tool-result")] ToolResult, - #[serde(rename = "tool-error")] - #[strum(serialize = "tool-error")] ToolError, } }crates/db-user/src/chat_messages_ops.rs (1)
45-49: *Harden reads against legacy NULLs; avoid SELECT .Coalesce type to the default and enumerate columns explicitly to prevent decode failures and insulate future schema changes.
Apply this diff:
- .query( - "SELECT * FROM chat_messages - WHERE group_id = ? - ORDER BY created_at ASC", - vec![group_id.into()], - ) + .query( + "SELECT + id, + group_id, + created_at, + role, + content, + COALESCE(type, 'text-delta') AS type + FROM chat_messages + WHERE group_id = ? + ORDER BY created_at ASC", + vec![group_id.into()], + )apps/desktop/src/components/settings/views/mcp.tsx (3)
85-96: Avoid persisting on every keystroke to the Tauri backend.handleUpdateServerHeader mutates and persists on each onChange, which can spam setServers and race mutations. Prefer debouncing, saving on blur, or a dedicated “Save” action to batch updates.
Example direction (no full refactor here):
- Update local state onChange.
- Trigger saveServersMutation onBlur or via an explicit Save button.
214-216: Use a stable key instead of the array index.Index keys can cause subtle UI issues on insert/delete. Use a stable identifier when possible (e.g., URL) or a composite.
- key={index} + key={`${server.url}-${index}`}
21-25: Trim “what” comments or add “why”.Comment on Line 21 states the obvious. Either remove or add rationale (e.g., why lazy-loading is required or any constraints).
apps/desktop/src/components/settings/components/tab-icon.tsx (1)
17-41: Avoid duplicate tab→icon mappings (centralize to TABS to prevent drift)You now define icons in both TABS and this switch. This easily drifts (e.g., some tabs already differ between files). Consider deriving the icon from TABS and rendering it here to keep a single source of truth.
Apply this diff to simplify TabIcon to use the TABS mapping (and avoid future inconsistencies):
import { type Tab } from "./types"; +import { TABS } from "./types"; export function TabIcon({ tab }: { tab: Tab }) { - switch (tab) { - case "general": - return <SettingsIcon className="h-4 w-4" />; - case "notifications": - return <BellIcon className="h-4 w-4" />; - case "sound": - return <AudioLinesIcon className="h-4 w-4" />; - case "feedback": - return <MessageSquareIcon className="h-4 w-4" />; - case "ai-llm": - return <SparklesIcon className="h-4 w-4" />; - case "ai-stt": - return <BirdIcon className="h-4 w-4" />; - case "calendar": - return <CalendarIcon className="h-4 w-4" />; - case "templates": - return <LayoutTemplateIcon className="h-4 w-4" />; - case "integrations": - return <BlocksIcon className="h-4 w-4" />; - case "billing": - return <CreditCardIcon className="h-4 w-4" />; - case "mcp": - return <NetworkIcon className="h-4 w-4" />; - default: - return null; - } + const Icon = TABS.find(t => t.name === tab)?.icon; + return Icon ? <Icon className="h-4 w-4" /> : null; }Note: You can then drop the individual icon imports at the top of this file.
apps/desktop/src/components/right-panel/components/chat/message-content.tsx (3)
126-152: Prefer Tailwind classes over inline styles; add aria-live for status updatesInline styles won’t adapt to themes/dark mode and are harder to keep consistent. Use utility classes and mark as live region so assistive tech announces tool events.
- return ( - <div - style={{ - backgroundColor: "rgb(250 250 250)", - border: "1px solid rgb(229 229 229)", - borderRadius: "6px", - padding: "12px 16px", - }} - > - <div - style={{ - color: "rgb(115 115 115)", - fontSize: "0.875rem", - display: "flex", - alignItems: "center", - gap: "8px", - }} - > - <PencilRuler size={16} color="rgb(115 115 115)" /> - <span style={{ fontWeight: "400" }}> - Called tool: {message.content} - </span> - </div> - </div> - ); + return ( + <div className="bg-neutral-50 border border-neutral-200 rounded-md px-4 py-3" aria-live="polite"> + <div className="flex items-center gap-2 text-neutral-500 text-sm"> + <PencilRuler size={16} className="text-neutral-500" /> + <span className="font-normal">Called tool: {message.content}</span> + </div> + </div> + );
154-180: Same here: switch to classes for theme consistencyUnify styling with utility classes to match the rest of the app.
- return ( - <div - style={{ - backgroundColor: "rgb(248 248 248)", - border: "1px solid rgb(224 224 224)", - borderRadius: "6px", - padding: "12px 16px", - }} - > - <div - style={{ - color: "rgb(115 115 115)", - fontSize: "0.875rem", - display: "flex", - alignItems: "center", - gap: "8px", - }} - > - <PencilRuler size={16} color="rgb(115 115 115)" /> - <span style={{ fontWeight: "400" }}> - {message.content} - </span> - </div> - </div> - ); + return ( + <div className="bg-neutral-50 border border-neutral-200 rounded-md px-4 py-3" aria-live="polite"> + <div className="flex items-center gap-2 text-neutral-500 text-sm"> + <PencilRuler size={16} className="text-neutral-500" /> + <span className="font-normal">{message.content}</span> + </div> + </div> + );
182-207: Use an error-specific icon/styling for tool-error; avoid inline stylesMinor UX nit: show an alert icon and error hues to differentiate error from normal tool output; also switch to classes.
-import { PencilRuler } from "lucide-react"; +import { PencilRuler, AlertCircle } from "lucide-react"; @@ - if (message.type === "tool-error") { - return ( - <div - style={{ - backgroundColor: "rgb(252 252 252)", - border: "1px solid rgb(229 229 229)", - borderRadius: "6px", - padding: "12px 16px", - }} - > - <div - style={{ - color: "rgb(115 115 115)", - fontSize: "0.875rem", - display: "flex", - alignItems: "center", - gap: "8px", - }} - > - <PencilRuler size={16} color="rgb(115 115 115)" /> - <span style={{ fontWeight: "400" }}> - Tool Error: {message.content} - </span> - </div> - </div> - ); - } + if (message.type === "tool-error") { + return ( + <div className="bg-red-50 border border-red-200 rounded-md px-4 py-3" aria-live="polite"> + <div className="flex items-center gap-2 text-red-600 text-sm"> + <AlertCircle size={16} className="text-red-600" /> + <span className="font-normal">Tool Error: {message.content}</span> + </div> + </div> + ); + }apps/desktop/src/components/right-panel/hooks/useChatQueries.ts (1)
99-107: Potential stale state after generation finishes; refetch once to sync DBWith the justFinishedGenerating guard, you skip syncing on the transition frame and rely on another change to retrigger. If the query data doesn’t change immediately, UI may remain stale. Refetch once when generation completes, and gate the log for dev only.
- if (chatMessagesQuery.data) { - if (!isGenerating && !justFinishedGenerating) { - // Safe to sync from database - setMessages(chatMessagesQuery.data); - setHasChatStarted(chatMessagesQuery.data.length > 0); - } else { - // Currently generating - DON'T override local state - console.log("Skipping DB sync - currently generating"); - } - } + if (chatMessagesQuery.data) { + if (!isGenerating && !justFinishedGenerating) { + // Safe to sync from database + setMessages(chatMessagesQuery.data); + setHasChatStarted(chatMessagesQuery.data.length > 0); + } else if (justFinishedGenerating) { + // Why: Force-refresh after completion so the next pass syncs fresh DB state + chatMessagesQuery.refetch(); + } else { + // Currently generating - DON'T override local state + if (process.env.NODE_ENV !== "production") { + console.debug("Skipping DB sync - currently generating"); + } + } + }apps/desktop/src/components/right-panel/components/chat/chat-messages-view.tsx (2)
14-41: Avoid injecting a <style> tag per render; prefer CSS class or TailwindEmbedding a <style> tag in a component leads to repeated style injection and harder maintenance. Move keyframes/classes to a stylesheet or Tailwind utilities.
For example, extract the CSS to a module or global stylesheet and keep ThinkingIndicator purely presentational. If you want, I can draft a Tailwind-friendly variant with equivalent dot animations.
110-112: Comment style: prefer “why” over “what”The comment describes what the indicator does. Consider either removing it (code is self-explanatory) or replacing with a brief “why” (e.g., “Debounce to avoid flicker on short-lived generating phases”).
Apply this diff to reword:
- {/* Thinking indicator with debounce - no flicker! */} + {/* Debounce to avoid flicker during short-lived generating states */}apps/desktop/src/components/right-panel/hooks/useMcpTools.ts (3)
6-8: Client cache could race under concurrent initializationsTwo concurrent callers may both miss the cache and create duplicate clients for the same URL. A promise cache (per-URL in-flight creation) would dedupe safely.
I can provide a follow-up diff that adds mcpClientPromiseCache: Map<string, Promise> and reuses it to serialize client creation per URL.
120-128: Close-and-clear to prevent stale cached clientscloseMcpClients closes clients but leaves them in the cache. Clearing them avoids reusing closed instances inadvertently.
Apply this diff:
export function closeMcpClients() { for (const client of mcpClientCache.values()) { try { client.close(); } catch (error) { console.error(`[MCP] Error closing client:`, error); } } + mcpClientCache.clear(); }
10-19: Excessive logging—gate behind a debug flagCurrent logs print server lists, tool names, and timings. Consider gating behind a debug flag to avoid noisy consoles and potential information exposure in production.
If desired, I can add a simple isDebug helper (e.g., process.env.NODE_ENV !== "production") and wrap logs.
apps/desktop/src/components/right-panel/utils/chat-utils.ts (3)
115-130: Shadowing outer sessionData creates confusionThe local const sessionData inside the mention.type === "note" branch shadows the outer parameter sessionData, harming readability and risking mistakes. Rename the inner variable.
Apply this diff:
- const sessionData = await dbCommands.getSession({ id: mention.id }); + const noteSession = await dbCommands.getSession({ id: mention.id }); - if (sessionData) { + if (noteSession) { - if (sessionData.enhanced_memo_html && sessionData.enhanced_memo_html.trim() !== "") { - noteContent = sessionData.enhanced_memo_html; + if (noteSession.enhanced_memo_html && noteSession.enhanced_memo_html.trim() !== "") { + noteContent = noteSession.enhanced_memo_html; - } else if (sessionData.raw_memo_html && sessionData.raw_memo_html.trim() !== "") { - noteContent = sessionData.raw_memo_html; + } else if (noteSession.raw_memo_html && noteSession.raw_memo_html.trim() !== "") { + noteContent = noteSession.raw_memo_html; } else { continue; }
71-87: System prompt logging may leak sensitive dataconsole.log("system prompt", systemContent) can expose user/session data in logs. Gate behind a dev/debug flag or remove.
Apply this diff:
- console.log("system prompt", systemContent); + if (import.meta.env?.DEV) { + console.debug("system prompt (dev)", systemContent); + }
41-46: Comment style: remove “✅ Add missing parameters”This comment is “what”-oriented and not needed. Prefer minimal “why”-style comments.
Apply this diff:
- // ✅ Add missing parametersapps/desktop/src/components/right-panel/hooks/useChatLogic.ts (6)
166-217: Duplicate MCP client/tool fetching logic—prefer centralization via useMcpToolsThis block re-implements MCP server discovery, client creation, and tool wrapping, duplicating the new useMcpTools hook. Duplication increases maintenance cost and may diverge (e.g., headers handling, caching).
Refactor to use useMcpTools for tool discovery and cache management, and only handle client lifecycle here if absolutely necessary (e.g., per-stream lifecycle).
365-406: Persisting AI text before tool-call is good—consider awaiting the savesaveAiText() is invoked without await. If the stream progresses quickly, the DB operation may race with subsequent writes. Awaiting here can simplify ordering guarantees at the cost of slight latency.
If ordering is important, await dbCommands.upsertChatMessage; otherwise leave as-is for throughput.
431-449: Tool error content may be non-stringchunk.error might be an object; string interpolation would produce “[object Object]”. Consider normalizing using String(...) or JSON.stringify with fallback.
Apply this diff:
- content: `Tool error: ${chunk.error}`, + content: `Tool error: ${typeof chunk.error === "string" ? chunk.error : JSON.stringify(chunk.error)}`,
125-143: User messages marked as type "text-delta" may be semantically misleadingUsing "text-delta" for user-authored messages can blur UI logic if “text-delta” is interpreted as streaming assistant output elsewhere. Consider a distinct type or omit type for user messages if not needed.
If you want, I can propose a small change to define a “user”/“assistant” semantic or add a “role” field consistently and use type only for assistant subtypes.
158-161: Production logs—gate or removeLogging llmConnection, model id, api base in production is noisy and may reveal environment details.
Wrap with a debug guard:
- console.log("llmConnection", llmConnection); - console.log("model id", model.modelId); - console.log("api base", apiBase); + if (import.meta.env?.DEV) { + console.debug("llmConnection", llmConnection); + console.debug("model id", model.modelId); + console.debug("api base", apiBase); + }
472-496: Minor: variable name typo finalErrorMesagefinalErrorMesage is misspelled; not harmful but worth correcting for readability.
Apply this diff:
- let finalErrorMesage = ""; + let finalErrorMessage = ""; @@ - finalErrorMesage = + finalErrorMessage = @@ - finalErrorMesage = "Sorry, I encountered an error. Please try again. " + "\n\n" + errorMsg; + finalErrorMessage = "Sorry, I encountered an error. Please try again. " + "\n\n" + errorMsg; @@ - content: finalErrorMesage, + content: finalErrorMessage, @@ - content: finalErrorMesage, + content: finalErrorMessage,
📜 Review details
Configuration used: .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 (26)
apps/desktop/src/components/editor-area/index.tsx(1 hunks)apps/desktop/src/components/right-panel/components/chat/chat-messages-view.tsx(3 hunks)apps/desktop/src/components/right-panel/components/chat/message-content.tsx(2 hunks)apps/desktop/src/components/right-panel/components/chat/types.ts(1 hunks)apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(7 hunks)apps/desktop/src/components/right-panel/hooks/useChatQueries.ts(2 hunks)apps/desktop/src/components/right-panel/hooks/useMcpTools.ts(1 hunks)apps/desktop/src/components/right-panel/utils/chat-utils.ts(2 hunks)apps/desktop/src/components/right-panel/views/chat-view.tsx(2 hunks)apps/desktop/src/components/settings/components/tab-icon.tsx(2 hunks)apps/desktop/src/components/settings/components/types.ts(3 hunks)apps/desktop/src/components/settings/views/index.ts(1 hunks)apps/desktop/src/components/settings/views/mcp.tsx(1 hunks)apps/desktop/src/locales/en/messages.po(10 hunks)apps/desktop/src/locales/ko/messages.po(10 hunks)apps/desktop/src/routes/app.settings.tsx(3 hunks)apps/desktop/src/utils/broadcast.ts(1 hunks)crates/db-user/src/chat_messages_migration_1.sql(1 hunks)crates/db-user/src/chat_messages_ops.rs(1 hunks)crates/db-user/src/chat_messages_types.rs(1 hunks)crates/db-user/src/init.rs(2 hunks)crates/db-user/src/lib.rs(2 hunks)crates/template/assets/ai_chat_system.jinja(1 hunks)crates/template/assets/enhance.system.jinja(2 hunks)packages/utils/src/ai.ts(1 hunks)plugins/db/js/bindings.gen.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/utils/broadcast.tsapps/desktop/src/components/right-panel/hooks/useMcpTools.tsapps/desktop/src/components/settings/components/tab-icon.tsxcrates/db-user/src/chat_messages_ops.rspackages/utils/src/ai.tsapps/desktop/src/components/right-panel/components/chat/types.tsapps/desktop/src/components/editor-area/index.tsxapps/desktop/src/components/settings/views/index.tscrates/db-user/src/chat_messages_types.rsapps/desktop/src/components/settings/views/mcp.tsxapps/desktop/src/routes/app.settings.tsxapps/desktop/src/components/settings/components/types.tsplugins/db/js/bindings.gen.tsapps/desktop/src/components/right-panel/components/chat/message-content.tsxapps/desktop/src/components/right-panel/views/chat-view.tsxapps/desktop/src/components/right-panel/hooks/useChatQueries.tscrates/db-user/src/lib.rsapps/desktop/src/components/right-panel/hooks/useChatLogic.tsapps/desktop/src/components/right-panel/utils/chat-utils.tsapps/desktop/src/components/right-panel/components/chat/chat-messages-view.tsxcrates/db-user/src/init.rs
🧬 Code Graph Analysis (7)
apps/desktop/src/components/right-panel/hooks/useMcpTools.ts (1)
packages/utils/src/ai.ts (3)
experimental_createMCPClient(10-10)tool(17-17)dynamicTool(9-9)
apps/desktop/src/components/settings/views/mcp.tsx (2)
plugins/mcp/js/bindings.gen.ts (3)
setServers(13-15)McpServer(28-28)commands(9-16)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop/src/routes/app.settings.tsx (1)
apps/desktop/src/components/settings/views/mcp.tsx (1)
MCP(11-285)
plugins/db/js/bindings.gen.ts (1)
apps/desktop/src/components/right-panel/components/chat/chat-message.tsx (1)
ChatMessage(11-48)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (6)
apps/desktop/src/components/right-panel/components/chat/types.ts (1)
Message(7-14)apps/desktop/src/components/right-panel/types/chat-types.ts (1)
Message(20-20)plugins/local-llm/src/server.rs (1)
content(267-273)packages/utils/src/ai.ts (6)
modelProvider(70-77)experimental_createMCPClient(10-10)tool(17-17)dynamicTool(9-9)streamText(16-16)stepCountIs(15-15)apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
prepareMessageHistory(35-202)apps/desktop/src/components/right-panel/utils/markdown-parser.ts (1)
parseMarkdownBlocks(3-62)
apps/desktop/src/components/right-panel/utils/chat-utils.ts (3)
apps/desktop/src/components/right-panel/types/chat-types.ts (1)
Message(20-20)packages/tiptap/src/editor/mention.tsx (1)
mention(259-316)apps/admin/src/lib/db/schema/auth.ts (1)
session(13-23)
crates/db-user/src/init.rs (1)
plugins/db/js/bindings.gen.ts (3)
ChatMessage(165-165)ChatMessageRole(166-166)ChatMessageType(167-167)
🪛 ast-grep (0.38.6)
apps/desktop/src/components/right-panel/utils/chat-utils.ts
[warning] 162-162: 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: div.innerHTML = session.enhanced_memo_html
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] 166-166: 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: div.innerHTML = session.raw_memo_html
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] 162-162: 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: div.innerHTML = session.enhanced_memo_html
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] 166-166: 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: div.innerHTML = session.raw_memo_html
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)
🔇 Additional comments (25)
apps/desktop/src/components/right-panel/components/chat/types.ts (1)
13-14: Validate DB & interop for updated Message.type union
The addition of"generating"makes sense for UI workflows, but it isn’t a persisted DB value. Please confirm that no producers or consumers of Message
– especially the parts that read/write from your store or backend – will break when encountering this new variant.• Review your database schema (or API definitions) to ensure only the four persisted types—
"text-delta" | "tool-start" | "tool-result" | "tool-error"—are stored or expected.
• Audit every site where a Message object is constructed or deserialized and verify that a validtypeis always set (or that there’s a safe default).
• Check code paths that handle incoming Message objects to ensure"generating"is treated appropriately (e.g. ignored/passthrough in persistence layers).
• Add or update unit/integration tests covering the new"generating"state and the defaulting behavior for Message.type.apps/desktop/src/locales/en/messages.po (1)
1029-1032: New translation key “MCP”: LGTMString and reference align with the new settings tab.
packages/utils/src/ai.ts (1)
8-18: No change needed forexperimental_createMCPClientexportConfirmed via the official Vercel AI SDK documentation that the exported symbol is indeed
experimental_createMCPClient(uppercase “MCP”). The current re-export matches the SDK and will compile correctly.crates/db-user/src/chat_messages_types.rs (2)
11-27: Good addition: typed message kinds with stable string mapping.Enum + serde/strum mapping aligns with bindings (TS: "text-delta" | "tool-start" | "tool-result" | "tool-error"). Cross-layer consistency looks solid.
36-37: Confirm nullability expectations for r#type.Deserialization will fail if DB returns NULL for “type”. Ensure the migration/backfill guarantees non-null values, or queries coalesce to a default.
crates/db-user/src/chat_messages_ops.rs (1)
12-20: INSERT now includes type: correct and consistent.Column order, placeholders, and value mapping (to_string via strum Display) are aligned with the new enum serialization.
crates/db-user/src/lib.rs (1)
131-156: Migrations updated correctly (append-only, count adjusted).Array length and placement look correct; new chat_messages migration is appended, honoring the append-only rule.
crates/db-user/src/init.rs (2)
4-6: Imports updated to include ChatMessageType — aligned with schema changes.The additional import matches the new DB column and enum wiring introduced elsewhere in the PR. Looks good.
791-798: Seeding ChatMessage with an explicit type is correct.Assigning r#type: ChatMessageType::TextDelta (Line 797) ensures seeds persist with the new discriminator and aligns with the DB default and frontend bindings.
plugins/db/js/bindings.gen.ts (1)
165-167: Type surface extended correctly (ChatMessage.type and ChatMessageType).The discriminated field addition is minimal and consistent with backend enums and UI render logic. No issues spotted.
apps/desktop/src/routes/app.settings.tsx (2)
72-74: Tab title mapping extended — LGTM.Adds MCP to getTabTitle switch. Matches the new settings view and keeps localization via t macro.
141-142: MCP tab wiring confirmed
All required pieces are present and in sync:
- apps/desktop/src/components/settings/components/types.ts:37
TABS includes{ name: "mcp", icon: NetworkIcon }- apps/desktop/src/components/settings/components/tab-icon.tsx:39
case "mcp": return <NetworkIcon …/>;- apps/desktop/src/components/settings/views/index.ts:8
export { default as MCP } from "./mcp";No further changes needed.
apps/desktop/src/components/settings/components/tab-icon.tsx (2)
10-10: Import of NetworkIcon is correct and usedThe new icon import is used below and aligns with the new MCP tab. No issues.
39-41: MCP tab icon wiring looks goodRendering NetworkIcon for the new "mcp" tab is consistent with the types/TABS definition.
apps/desktop/src/components/settings/views/index.ts (1)
8-8: Export for MCP view added correctlyThe index now re-exports MCP, enabling the route to import it from a single barrel. Looks good.
apps/desktop/src/components/settings/components/types.ts (3)
9-9: NetworkIcon import is correct and usedThe icon is referenced in TABS for the "mcp" tab; no unused imports.
25-25: Tab union extended with "mcp"Type surface updated cleanly; downstream code can now rely on the discriminated tab.
37-37: MCP added to TABS with NetworkIconThe new tab is discoverable by any UI that iterates TABS. Consistent with TabIcon.
apps/desktop/src/components/right-panel/components/chat/message-content.tsx (1)
3-3: New PencilRuler icon import is usedImport matches usage in the new tool message UI blocks.
apps/desktop/src/components/right-panel/hooks/useChatQueries.ts (1)
87-89: No normalization needed: ChatMessageType is already a lowercase hyphenated union
Theplugins/db/js/bindings.gen.tsclearly definesexport type ChatMessageType = "text-delta" | "tool-start" | "tool-result" | "tool-error"so
msg.typewill always match the expected string union. No additional runtime normalization is required here.apps/desktop/src/components/right-panel/views/chat-view.tsx (2)
62-63: Prop plumbing looks correctForwarding isStreamingText from useChatLogic and passing both isGenerating and isStreamingText down to ChatMessagesView is consistent with the new streaming UI. No issues spotted.
169-176: No other usages of ChatMessagesView found
A project-wide search revealed only the single call site in
apps/desktop/src/components/right-panel/views/chat-view.tsx
on lines 169–176. There are no other TSX files invoking , so no further prop-alignment updates are needed elsewhere.apps/desktop/src/components/right-panel/components/chat/chat-messages-view.tsx (1)
50-70: Edge cases in thinking logic—tool chunks vs. streamingshouldShowThinking returns true whenever the last message is not from the user and isStreamingText is false. If the last message is a tool-* message (tool-start/result/error), this will show “Thinking,” which might be intended. If not, consider checking message.type to gate the indicator (e.g., only when last is assistant text or after a user message).
Would you like me to adjust the predicate to be type-aware (e.g., ignore tool-result/error as “thinking” states)?
apps/desktop/src/components/right-panel/hooks/useMcpTools.ts (1)
26-31: No direct consumers found—please verify downstream usage
I ran a repository-wide search foruseMcpTools(in.tsand.tsxfiles and didn’t find any call sites. That means there are currently no consumers to break, but be sure that any future components using this hook handle its return value as an object map (Record<string, any>) and not an array.apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (1)
297-308: Ensure generating flags are reset on finish as wellonFinish closes clients; flags are reset after the for-await loop (good). Just calling out that if onFinish fires before the loop naturally ends in all providers, there’s no additional cleanup here. Keep an eye on provider semantics.
If you see orphaned “Thinking” indicators after finish, consider toggling setIsGenerating(false)/setIsStreamingText(false) in onFinish as a safety.
| const thinkingTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
|
|
There was a problem hiding this comment.
Fix type for thinkingTimeoutRef to avoid TS/browser typing mismatch
NodeJS.Timeout is incorrect in browser React apps and may cause typing errors. Use ReturnType.
Apply this diff:
- const thinkingTimeoutRef = useRef<NodeJS.Timeout | null>(null);
+ const thinkingTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const thinkingTimeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| const thinkingTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); |
🤖 Prompt for AI Agents
In
apps/desktop/src/components/right-panel/components/chat/chat-messages-view.tsx
around lines 48-49, the thinkingTimeoutRef is typed as NodeJS.Timeout | null
which mismatches browser setTimeout types; change its type to ReturnType<typeof
setTimeout> | null (i.e. use ReturnType<typeof setTimeout> instead of
NodeJS.Timeout) so the ref matches the DOM/browser timer return type and avoid
TS errors.
| if (noteContents.length > 0) { | ||
| currentUserMessage = currentUserMessage + noteContents.join(""); | ||
| } | ||
| } |
There was a problem hiding this comment.
Same bug when concatenating noteContents
Guard against undefined here as well.
Apply this diff:
- if (noteContents.length > 0) {
- currentUserMessage = currentUserMessage + noteContents.join("");
- }
+ if (noteContents.length > 0) {
+ currentUserMessage = (currentUserMessage ?? "") + noteContents.join("");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (noteContents.length > 0) { | |
| currentUserMessage = currentUserMessage + noteContents.join(""); | |
| } | |
| } | |
| if (noteContents.length > 0) { | |
| currentUserMessage = (currentUserMessage ?? "") + noteContents.join(""); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/utils/chat-utils.ts around lines 189
to 192, the code concatenates noteContents without guarding against it being
undefined; update the logic to first verify noteContents is defined and an array
(or coerce to an empty array) before checking length and joining — e.g. treat
noteContents as (noteContents || []) or add a conditional if
(Array.isArray(noteContents) && noteContents.length > 0) then append
noteContents.join("") to currentUserMessage so the concatenation won’t throw
when noteContents is undefined.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
6 issues found across 26 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| <div className="space-y-3"> | ||
| {servers.map((server, index) => ( | ||
| <div | ||
| key={index} |
There was a problem hiding this comment.
Using the array index as the React key can break component identity when servers are added or removed, leading to incorrect re-rendering or stale state. A stable, unique property such as the server URL should be used instead.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/settings/views/mcp.tsx at line 215:
<comment>Using the array index as the React key can break component identity when servers are added or removed, leading to incorrect re-rendering or stale state. A stable, unique property such as the server URL should be used instead.</comment>
<file context>
@@ -0,0 +1,285 @@
+import { commands, type McpServer } from "@hypr/plugin-mcp";
+import { Button } from "@hypr/ui/components/ui/button";
+import { Input } from "@hypr/ui/components/ui/input";
+import { Label } from "@hypr/ui/components/ui/label";
+import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@hypr/ui/components/ui/select";
+import { Switch } from "@hypr/ui/components/ui/switch";
+import { useMutation } from "@tanstack/react-query";
+import { PlusIcon, Trash2Icon } from "lucide-react";
+import { useEffect, useState } from "react";
</file context>
apps/desktop/src/components/right-panel/components/chat/message-content.tsx
Show resolved
Hide resolved
| " | ||
|
|
||
| {% if modelId == "gpt-4.1" or modelId == "openai/gpt-4.1" or modelId == "anthropic/claude-4-sonnet" or modelId == "openai/gpt-4o" or modelId == "gpt-4o"%} | ||
| {% if modelId == "gpt-4.1" or modelId == "openai/gpt-4.1" or modelId == "anthropic/claude-sonnet-4" or modelId == "openai/gpt-4o" or modelId == "gpt-4o" or "pro.hyprnote.com" in apiBase%} |
There was a problem hiding this comment.
If apiBase is undefined or None, the membership test "pro.hyprnote.com" in apiBase raises a TypeError at render time, breaking the whole template. Guard the test with a truthiness check on apiBase first.
Prompt for AI agents
Address the following comment on crates/template/assets/ai_chat_system.jinja at line 147:
<comment>If apiBase is undefined or None, the membership test "pro.hyprnote.com" in apiBase raises a TypeError at render time, breaking the whole template. Guard the test with a truthiness check on apiBase first.</comment>
<file context>
@@ -144,10 +144,17 @@ Your response would mostly be either of the two formats:
"
-{% if modelId == "gpt-4.1" or modelId == "openai/gpt-4.1" or modelId == "anthropic/claude-4-sonnet" or modelId == "openai/gpt-4o" or modelId == "gpt-4o"%}
+{% if modelId == "gpt-4.1" or modelId == "openai/gpt-4.1" or modelId == "anthropic/claude-sonnet-4" or modelId == "openai/gpt-4o" or modelId == "gpt-4o" or "pro.hyprnote.com" in apiBase%}
[Tool Calling]
Here are available tools that you can call to get more information.
</file context>
| {% if modelId == "gpt-4.1" or modelId == "openai/gpt-4.1" or modelId == "anthropic/claude-sonnet-4" or modelId == "openai/gpt-4o" or modelId == "gpt-4o" or "pro.hyprnote.com" in apiBase%} | |
| {% if modelId == "gpt-4.1" or modelId == "openai/gpt-4.1" or modelId == "anthropic/claude-sonnet-4" or modelId == "openai/gpt-4o" or modelId == "gpt-4o" or (apiBase and "pro.hyprnote.com" in apiBase) %} |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
crates/template/assets/ai_chat_system.jinja (2)
147-147: Good guard on apiBase and updated model gatingThe additional
(apiBase and "pro.hyprnote.com" in apiBase)prevents a TypeError when apiBase is falsy and broadens tool enablement as intended.
151-151: Typo: “producitvity” → “productivity”Fix the spelling in user-facing instructions.
Apply this diff:
- - custom MCP tools: user will be able to request you to call MCP tools. Often times, they are related to other producitvity tools like Slack, Notion, Google workspace, etc. + - custom MCP tools: user will be able to request you to call MCP tools. Often times, they are related to other productivity tools like Slack, Notion, Google Workspace, etc.apps/desktop/src/components/right-panel/utils/chat-utils.ts (4)
86-86: Remove debug log from production pathLeaking the full system prompt clutters logs and can expose sensitive user data. Remove or gate behind a DEV flag.
- console.log("system prompt", systemContent); + // Consider enabling only in development: + // if (import.meta?.env?.DEV) console.debug("system prompt", systemContent);
102-106: Fix undefined concatenation when appending mention prefaceUsing += on possibly undefined currentUserMessage coerces it to "undefined…".
- if (mentionedContent && mentionedContent.length > 0) { - currentUserMessage += - "[[From here is an automatically appended content from the mentioned notes & people, not what the user wrote. Use this only as a reference for more context. Your focus should always be the current meeting user is viewing]]" - + "\n\n"; - } + if (mentionedContent && mentionedContent.length > 0) { + const preface = + "[[From here is an automatically appended content from the mentioned notes & people, not what the user wrote. Use this only as a reference for more context. Your focus should always be the current meeting user is viewing]]\n\n"; + currentUserMessage = (currentUserMessage ?? "") + preface; + }
158-168: Avoid assigning unsanitized HTML to innerHTML; parse safely to textDirect innerHTML assignment is unnecessary and risks XSS if this ever runs in a context where HTML executes. Use DOMParser and read textContent.
- if (session.enhanced_memo_html && session.enhanced_memo_html.trim() !== "") { - const div = document.createElement("div"); - div.innerHTML = session.enhanced_memo_html; - briefContent = (div.textContent || div.innerText || "").slice(0, 200) + "..."; - } else if (session.raw_memo_html && session.raw_memo_html.trim() !== "") { - const div = document.createElement("div"); - div.innerHTML = session.raw_memo_html; - briefContent = (div.textContent || div.innerText || "").slice(0, 200) + "..."; - } + if (session.enhanced_memo_html && session.enhanced_memo_html.trim() !== "") { + const parsed = new DOMParser().parseFromString(session.enhanced_memo_html, "text/html"); + briefContent = (parsed.body.textContent || "").slice(0, 200) + "..."; + } else if (session.raw_memo_html && session.raw_memo_html.trim() !== "") { + const parsed = new DOMParser().parseFromString(session.raw_memo_html, "text/html"); + briefContent = (parsed.body.textContent || "").slice(0, 200) + "..."; + }
188-191: Guard against undefined when appending aggregated note contentsAvoid "undefined..." prefix by using nullish coalescing.
- if (noteContents.length > 0) { - currentUserMessage = currentUserMessage + noteContents.join(""); - } + if (noteContents.length > 0) { + currentUserMessage = (currentUserMessage ?? "") + noteContents.join(""); + }
🧹 Nitpick comments (5)
apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
118-128: Strip HTML before injecting note content into the prompt to reduce token bloatYou currently inject raw HTML into the LLM prompt. Convert to plaintext to keep prompts clean and avoid markup artifacts.
- noteContents.push(`\n\n--- Content from the note"${mention.label}" ---\n${noteContent}`); + const parsedNote = new DOMParser().parseFromString(noteContent, "text/html"); + const plainText = parsedNote.body.textContent ?? ""; + noteContents.push(`\n\n--- Content from the note "${mention.label}" ---\n${plainText}`);apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (4)
169-173: Nit: fix typo in variable nameUse enabledServers for clarity.
- const enabledSevers = mcpServers.filter((server) => server.enabled); + const enabledServers = mcpServers.filter((server) => server.enabled); - for (const server of enabledSevers) { + for (const server of enabledServers) {
193-201: Avoid shadowing the importedtoolsymbol; also build the prompt’s tool list from source definitionsThe loop variable named
toolshadows the importedtool()helper. Rename to avoid confusion and capture inputSchema/description directly from the source tool.- const tools = await mcpClient.tools(); - for (const [toolName, tool] of Object.entries(tools as Record<string, any>)) { - newMcpTools[toolName] = dynamicTool({ - description: tool.description, - inputSchema: tool.inputSchema || z.any(), - execute: tool.execute, - }); - } + const tools = await mcpClient.tools(); + for (const [toolName, srvTool] of Object.entries(tools as Record<string, any>)) { + // Register executable tool for the model + newMcpTools[toolName] = dynamicTool({ + description: srvTool.description, + inputSchema: srvTool.inputSchema || z.any(), + execute: srvTool.execute, + }); + // Also accumulate a prompt-facing summary with original schema + mcpToolsArray.push({ + name: toolName, + description: srvTool.description || `Tool: ${toolName}`, + inputSchema: srvTool.inputSchema ?? null, + }); + }
183-189: Gate SSE client logs behind a DEV flagAvoid noisy console logs in production.
- onerror: (error) => { - console.log("mcp client error: ", error); - }, - onclose: () => { - console.log("mcp client closed"); - }, + onerror: (error) => { + if (import.meta?.env?.DEV) console.debug("mcp client error:", error); + }, + onclose: () => { + if (import.meta?.env?.DEV) console.debug("mcp client closed"); + },
58-59: isGeneratingRef appears unnecessaryYou set isGeneratingRef.current but never read it. Remove to reduce noise unless you plan to use it for cancellation.
- const [isStreamingText, setIsStreamingText] = useState(false); - const isGeneratingRef = useRef(false); + const [isStreamingText, setIsStreamingText] = useState(false);Follow-ups within this file:
- Remove writes to isGeneratingRef.current at lines 121, 296, 464, 482.
📜 Review details
Configuration used: .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 (4)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(7 hunks)apps/desktop/src/components/right-panel/hooks/useMcpTools.ts(1 hunks)apps/desktop/src/components/right-panel/utils/chat-utils.ts(2 hunks)crates/template/assets/ai_chat_system.jinja(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/right-panel/hooks/useMcpTools.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/hooks/useChatLogic.tsapps/desktop/src/components/right-panel/utils/chat-utils.ts
🧬 Code Graph Analysis (2)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (5)
apps/desktop/src/components/right-panel/components/chat/types.ts (1)
Message(7-14)apps/desktop/src/components/right-panel/types/chat-types.ts (1)
Message(20-20)packages/utils/src/ai.ts (2)
modelProvider(70-77)tool(17-17)apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
prepareMessageHistory(35-201)apps/desktop/src/components/right-panel/utils/markdown-parser.ts (1)
parseMarkdownBlocks(3-62)
apps/desktop/src/components/right-panel/utils/chat-utils.ts (3)
apps/desktop/src/components/right-panel/types/chat-types.ts (1)
Message(20-20)packages/tiptap/src/editor/mention.tsx (1)
mention(259-316)apps/admin/src/lib/db/schema/auth.ts (1)
session(13-23)
🪛 ast-grep (0.38.6)
apps/desktop/src/components/right-panel/utils/chat-utils.ts
[warning] 161-161: 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: div.innerHTML = session.enhanced_memo_html
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] 165-165: 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: div.innerHTML = session.raw_memo_html
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] 161-161: 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: div.innerHTML = session.enhanced_memo_html
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] 165-165: 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: div.innerHTML = session.raw_memo_html
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 (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (2)
apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
70-74: Timeline filter expects a JSON string,JSON.stringifyis correct
Thetimelinefilter incrates/template/src/filters.rsis defined as:pub fn timeline(words: String) -> String { let words: Vec<Word2> = serde_json::from_str(&words).unwrap(); … }It explicitly deserializes a JSON string into a
Vec. Passing an array directly would break at runtime. Keep the current usage:words: JSON.stringify(freshSessionData?.words || []),Likely an incorrect or invalid review comment.
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (1)
162-167: MCP tool gating logic LGTMChecks both supported model IDs and apiBase, and excludes HyprLocal. Matches the system prompt gating.
| mcpToolsArray = Object.keys(newMcpTools).length > 0 | ||
| ? Object.entries(newMcpTools).map(([name, tool]) => ({ | ||
| name, | ||
| description: tool.description || `Tool: ${name}`, | ||
| inputSchema: tool.inputSchema || "No input schema provided", | ||
| })) | ||
| : []; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t rebuild mcpToolsArray from wrapped tools; keep original schema/description
The dynamic wrappers may not expose inputSchema cleanly and can lead to [object Object] in the prompt. You already accumulate summaries above; remove this block.
- mcpToolsArray = Object.keys(newMcpTools).length > 0
- ? Object.entries(newMcpTools).map(([name, tool]) => ({
- name,
- description: tool.description || `Tool: ${name}`,
- inputSchema: tool.inputSchema || "No input schema provided",
- }))
- : [];
+ // mcpToolsArray is already populated from source tool definitions📝 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.
| mcpToolsArray = Object.keys(newMcpTools).length > 0 | |
| ? Object.entries(newMcpTools).map(([name, tool]) => ({ | |
| name, | |
| description: tool.description || `Tool: ${name}`, | |
| inputSchema: tool.inputSchema || "No input schema provided", | |
| })) | |
| : []; | |
| } | |
| // mcpToolsArray is already populated from source tool definitions | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/hooks/useChatLogic.ts around lines
206 to 213, remove the block that rebuilds mcpToolsArray from newMcpTools (the
Object.keys/Object.entries mapping) because wrappers can hide
inputSchema/description and produce [object Object]; do not reassign or rebuild
mcpToolsArray here—keep and use the original accumulated summaries collected
earlier, so the prompt uses the original schema/description values instead of
reconstructed objects.
| {% for tool in mcpTools %} | ||
|
|
||
| - {{ tool.name }}: {{ tool.description }}. Input schema is {{ tool.inputSchema }}. | ||
| {% endfor %} |
There was a problem hiding this comment.
Guard against undefined mcpTools and truncate verbose schemas
Iterating over an undefined mcpTools can error at render-time. Also, dumping full inputSchema can bloat prompts.
Apply this diff to make the loop safe and reduce token usage:
-{% for tool in mcpTools %}
+{% for tool in (mcpTools | default([])) %}
-- {{ tool.name }}: {{ tool.description }}. Input schema is {{ tool.inputSchema }}.
+- {{ tool.name }}: {{ tool.description }}.{% if tool.inputSchema %} Input schema (truncated): {{ tool.inputSchema | truncate(300) }}{% endif %}
{% endfor %}Optionally, if your Jinja environment supports tojson, prefer:
- Input schema (truncated): {{ tool.inputSchema | truncate(300) }}
+ Input schema (truncated): {{ tool.inputSchema | tojson | truncate(300) }}📝 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.
| {% for tool in mcpTools %} | |
| - {{ tool.name }}: {{ tool.description }}. Input schema is {{ tool.inputSchema }}. | |
| {% endfor %} | |
| {% for tool in (mcpTools | default([])) %} | |
| - {{ tool.name }}: {{ tool.description }}{% if tool.inputSchema %} Input schema (truncated): {{ tool.inputSchema | truncate(300) }}{% endif %} | |
| {% endfor %} |
🤖 Prompt for AI Agents
In crates/template/assets/ai_chat_system.jinja around lines 154 to 157, the
template currently iterates mcpTools directly and prints full inputSchema which
can error if mcpTools is undefined and can bloat prompts; update the loop to
first guard against undefined/empty mcpTools (e.g., default to empty list or use
an if-statement) and iterate only when present, and truncate or summarize
tool.inputSchema (limit to a short substring or replace with a brief
type/summary) to reduce tokens; if your Jinja environment supports tojson,
prefer rendering a compact JSON snippet via tojson for the schema output instead
of dumping the full object.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/desktop/src/components/settings/components/ai/llm-local-view.tsx (5)
220-225: Remove debug logs from the click handlerKeeps the UI clean and avoids noisy consoles in production.
- onClick={(e) => { - console.log("model download clicked"); - e.stopPropagation(); - console.log("model download clicked 2"); - handleModelDownload(model.key); - }} + onClick={(e) => { + e.stopPropagation(); + handleModelDownload(model.key); + }}
201-203: Use OS-agnostic label“Show in Finder” is macOS-specific; “Show in Folder” is clearer cross-platform.
- Show in Finder + Show in Folder
39-41: Async function without awaitEither remove
asyncor useawaitfor clarity. Prefer the latter for readability.- const handleShowFileLocation = async () => { - localLlmCommands.modelsDir().then((path) => openPath(path)); - }; + const handleShowFileLocation = async () => { + const path = await localLlmCommands.modelsDir(); + await openPath(path); + };
45-47: Avoid redundant state updates in effectGuarding avoids unnecessary re-renders if the selected model is already current.
- if (currentLLMModel.data && !customLLMEnabled.data) { - setSelectedLLMModel(currentLLMModel.data); - } + if (currentLLMModel.data && !customLLMEnabled.data) { + setSelectedLLMModel((prev) => + prev === currentLLMModel.data ? prev : currentLLMModel.data, + ); + }
59-66: No need for additional verification—side-effects are limited to query refetches, and mutateAsync is available. Converting both toggles tomutateAsyncwithinPromise.allwill batch-disable flags before restarting the server, eliminating flicker without breaking onSuccess handlers.Update
handleLocalModelSelectionto atomically disable both flags// Disable BOTH HyprCloud and custom when selecting local - setCustomLLMEnabledMutation.mutate(false); - setHyprCloudEnabledMutation.mutate(false); + await Promise.all([ + setCustomLLMEnabledMutation.mutateAsync(false), + setHyprCloudEnabledMutation.mutateAsync(false), + ]); setOpenAccordion(null); // Restart server for local model localLlmCommands.restartServer();
📜 Review details
Configuration used: .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 (2)
apps/desktop/src/components/settings/components/ai/llm-local-view.tsx(1 hunks)apps/desktop/src/components/settings/components/tab-icon.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/settings/components/tab-icon.tsx
🧰 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/settings/components/ai/llm-local-view.tsx
⏰ 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 (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
1 participant
don't merge yet - work in progress
Summary by cubic
Adds MCP tool calling to chat with a new MCP settings tab, improved streaming UX, and persistent tool event messages. Adds a database column to store message types and updates prompts to reflect tool availability.
New Features
Migration