From 0a1f0c2e78e0929931be46823530613a7fb3a0d7 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 25 Mar 2026 10:47:17 +0000 Subject: [PATCH] fix(chat): address review feedback from Gemini, Codex, and internal review 1. ArtifactPanel textarea: use local state with onChange/onBlur instead of empty onChange handler that made the editor unwritable 2. Artifact emission: move store.addArtifact from render body into a dedicated ArtifactEmitter component using useEffect 3. searchText for new threads: write searchText during generateTitle so post-migration threads are searchable by content 4. PromptLibrarySheet: wire onInsertPrompt with clipboard fallback and toast notification; handle prompt() cancel (null) as abort 5. download.ts: defer URL.revokeObjectURL with setTimeout to avoid failed downloads on Firefox 6. DB migration: replace serial for-of await loop with Promise.all to prevent transaction timeout on large databases; skip empty text 7. handleDeleteFolder: use Dexie transaction with .modify() for atomic unfile-then-delete; use undefined instead of "" for unfiled folderId 8. Keyboard shortcuts: remove Cmd+Shift+F from help dialog since the handler is not wired Also: - Remove unused aui hook, SearchIcon import, and prompt-library imports - Fix stale artifactPanelOpen closure in command palette toggle - Fix JSONL export: remove non-standard feedback field from message objects; use application/x-ndjson MIME type - Fix nested button in ArtifactTab: use div with role="tab" - Use static db import instead of redundant dynamic import in adapter - Log memory injection errors instead of silent catch - Add cancelled flag to feedback useEffect to prevent stale setState - Use functional setState in handleFeedback to avoid stale closure - Add .catch() on feedback DB write --- .../components/assistant-ui/markdown-text.tsx | 58 ++++++++++--------- .../src/components/assistant-ui/thread.tsx | 24 ++++---- .../src/features/chat/api/chat-adapter.ts | 7 +-- .../frontend/src/features/chat/chat-page.tsx | 8 ++- .../chat/components/artifact-panel.tsx | 36 +++++++----- .../components/keyboard-shortcut-help.tsx | 1 - .../chat/components/prompt-library-sheet.tsx | 6 +- studio/frontend/src/features/chat/db.ts | 33 ++++++----- .../src/features/chat/lib/thread-export.ts | 5 +- .../src/features/chat/runtime-provider.tsx | 5 ++ .../src/features/chat/thread-sidebar.tsx | 15 +++-- studio/frontend/src/lib/download.ts | 2 +- 12 files changed, 111 insertions(+), 89 deletions(-) diff --git a/studio/frontend/src/components/assistant-ui/markdown-text.tsx b/studio/frontend/src/components/assistant-ui/markdown-text.tsx index 7c14aa150a..2e31e7f061 100644 --- a/studio/frontend/src/components/assistant-ui/markdown-text.tsx +++ b/studio/frontend/src/components/assistant-ui/markdown-text.tsx @@ -12,7 +12,7 @@ import { code } from "@streamdown/code"; import { createMathPlugin } from "@streamdown/math"; import { mermaid } from "@streamdown/mermaid"; import { DownloadIcon, Maximize2Icon, Minimize2Icon } from "lucide-react"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Block, type BlockProps, Streamdown } from "streamdown"; import "katex/dist/katex.min.css"; import { AudioPlayer } from "./audio-player"; @@ -337,6 +337,23 @@ function CodeBlockActions({ ); } +/** Emits an artifact to the store via useEffect (avoids side-effects in render). */ +function ArtifactEmitter({ language, source }: { language: string | null; source: string }) { + useEffect(() => { + const artifactId = `artifact-${hashCode(source)}`; + const store = useArtifactStore.getState(); + if (store.artifacts.some((a) => a.id === artifactId)) return; + store.addArtifact({ + id: artifactId, + title: language ? `${language} snippet` : "Code snippet", + language, + content: source, + createdAt: Date.now(), + }); + }, [language, source]); + return null; +} + function StreamdownBlock(props: BlockProps) { const hasMermaidFence = props.content.includes("```mermaid"); const mermaidSource = getMermaidSource(props.content); @@ -389,34 +406,23 @@ function StreamdownBlock(props: BlockProps) { const isArtifactWorthy = !props.isIncomplete && (lineCount >= 20 || svgSource !== null || htmlSource !== null); - if (isArtifactWorthy) { - const artifactId = `artifact-${hashCode(codeFence.source)}`; - const store = useArtifactStore.getState(); - if (!store.artifacts.some((a) => a.id === artifactId)) { - store.addArtifact({ - id: artifactId, - title: codeFence.language - ? `${codeFence.language} snippet` - : "Code snippet", - language: codeFence.language, - content: codeFence.source, - createdAt: Date.now(), - }); - } - } return ( <> -
- - -
- {svgSource && } - {htmlSource && } + {isArtifactWorthy && ( + + )} + +
+ + +
+ {svgSource && } + {htmlSource && } ); } diff --git a/studio/frontend/src/components/assistant-ui/thread.tsx b/studio/frontend/src/components/assistant-ui/thread.tsx index eccd8b6716..cb3720f89f 100644 --- a/studio/frontend/src/components/assistant-ui/thread.tsx +++ b/studio/frontend/src/components/assistant-ui/thread.tsx @@ -619,7 +619,6 @@ const CopyButton: FC = () => { }; const FeedbackButtons: FC = () => { - const aui = useAui(); const messageId = useAuiState(({ message }) => message.id); const [feedback, setFeedback] = useState<"thumbs_up" | "thumbs_down" | null>( null, @@ -628,22 +627,27 @@ const FeedbackButtons: FC = () => { // Load existing feedback from DB useEffect(() => { if (!messageId) return; + setFeedback(null); + let cancelled = false; void db.messages.get(messageId).then((msg) => { - if (msg?.feedback) setFeedback(msg.feedback); + if (!cancelled && msg?.feedback) setFeedback(msg.feedback); }); + return () => { cancelled = true; }; }, [messageId]); const handleFeedback = useCallback( (value: "thumbs_up" | "thumbs_down") => { - const next = feedback === value ? null : value; - setFeedback(next); - if (messageId) { - void db.messages.update(messageId, { - feedback: next ?? undefined, - }); - } + setFeedback((prev) => { + const next = prev === value ? null : value; + if (messageId) { + void db.messages + .update(messageId, { feedback: next ?? undefined }) + .catch((err) => console.error("Failed to save feedback:", err)); + } + return next; + }); }, - [feedback, messageId], + [messageId], ); return ( diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index 1f5f6b10b8..845b79df59 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -446,8 +446,7 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { let systemContent = typeof params.systemPrompt === "string" ? params.systemPrompt.trim() : ""; try { - const { db: chatDb } = await import("../db"); - const allMemories = await chatDb.memory.toArray(); + const allMemories = await db.memory.toArray(); const enabledMemories = allMemories.filter( (m: { enabled: boolean }) => m.enabled, ); @@ -461,8 +460,8 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { ? `${memoryBlock}\n\n${systemContent}` : memoryBlock; } - } catch { - // Memory table may not exist yet during migration + } catch (err) { + console.warn("Memory injection skipped:", err); } if (systemContent) { outboundMessages.unshift({ diff --git a/studio/frontend/src/features/chat/chat-page.tsx b/studio/frontend/src/features/chat/chat-page.tsx index 8a796a23e7..6838b47a78 100644 --- a/studio/frontend/src/features/chat/chat-page.tsx +++ b/studio/frontend/src/features/chat/chat-page.tsx @@ -27,6 +27,7 @@ import { SheetTitle, } from "@/components/ui/sheet"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; +import { copyToClipboard } from "@/lib/copy-to-clipboard"; import { cn } from "@/lib/utils"; import { ColumnInsertIcon, @@ -41,7 +42,6 @@ import { KeyboardIcon, PanelRightIcon, PencilIcon, - SearchIcon, SettingsIcon, } from "lucide-react"; import { @@ -775,6 +775,10 @@ export function ChatPage(): ReactElement { { + copyToClipboard(content); + toast.success("Prompt copied to clipboard"); + }} /> @@ -805,7 +809,7 @@ export function ChatPage(): ReactElement { Prompt Library - { useArtifactStore.getState().setPanelOpen(!artifactPanelOpen); setCommandPaletteOpen(false); }}> + { const s = useArtifactStore.getState(); s.setPanelOpen(!s.panelOpen); setCommandPaletteOpen(false); }}> Toggle Artifacts Panel diff --git a/studio/frontend/src/features/chat/components/artifact-panel.tsx b/studio/frontend/src/features/chat/components/artifact-panel.tsx index 8d3fe50562..c2a08bbbe8 100644 --- a/studio/frontend/src/features/chat/components/artifact-panel.tsx +++ b/studio/frontend/src/features/chat/components/artifact-panel.tsx @@ -12,7 +12,7 @@ import { DownloadIcon, XIcon, } from "lucide-react"; -import { type FC, useRef, useState } from "react"; +import { type FC, useEffect, useRef, useState } from "react"; import { type Artifact, useArtifactStore, @@ -28,10 +28,12 @@ const ArtifactTab: FC<{ artifact: Artifact; isActive: boolean }> = ({ const remove = useArtifactStore((s) => s.removeArtifact); return ( - - + ); }; @@ -61,13 +63,18 @@ export const ArtifactPanel: FC = () => { const updateContent = useArtifactStore((s) => s.updateArtifactContent); const [copied, setCopied] = useState(false); + const [localValue, setLocalValue] = useState(""); const resetRef = useRef>(); const textareaRef = useRef(null); - if (!panelOpen || artifacts.length === 0) return null; - const active = artifacts.find((a) => a.id === activeId) ?? artifacts[0]; - if (!active) return null; + + // Sync local editor value when active artifact changes + useEffect(() => { + if (active) setLocalValue(active.content); + }, [active?.id, active?.content]); + + if (!panelOpen || artifacts.length === 0 || !active) return null; const handleCopy = () => { if (copyToClipboard(active.content)) { @@ -152,14 +159,11 @@ export const ArtifactPanel: FC = () => {