From 007ffd25696986cc4f1994095b49f3de99660b3c Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 31 Mar 2026 11:28:04 +0000 Subject: [PATCH] Fix review findings for chat UI enhancements - artifact-panel: add initial value to useRef (fixes TS2554), sync editor on version navigation, use localValue for copy/download, cleanup timer - thread.tsx: use Dexie modify+delete to properly clear feedback field - thread-sidebar: use .filter()/.some() for compare-thread search, use Dexie modify+delete to properly unfile threads from folders - thread-export: add model suffix to compare-export filenames, support Unicode in sanitizeFilename - db.ts: use sequential for-of loop instead of Promise.all in v4 upgrade to avoid IndexedDB transaction auto-commit - download.ts: increase revocation delay to 1s, wrap in try/finally - chat-page: clear artifact store on thread switch --- .../src/components/assistant-ui/thread.tsx | 15 ++++-- .../frontend/src/features/chat/chat-page.tsx | 2 + .../chat/components/artifact-panel.tsx | 28 +++++++---- studio/frontend/src/features/chat/db.ts | 49 +++++++++---------- .../src/features/chat/lib/thread-export.ts | 19 +++++-- .../src/features/chat/thread-sidebar.tsx | 30 ++++++++---- studio/frontend/src/lib/download.ts | 17 ++++--- 7 files changed, 101 insertions(+), 59 deletions(-) diff --git a/studio/frontend/src/components/assistant-ui/thread.tsx b/studio/frontend/src/components/assistant-ui/thread.tsx index 3e57f7b480..def4a3d373 100644 --- a/studio/frontend/src/components/assistant-ui/thread.tsx +++ b/studio/frontend/src/components/assistant-ui/thread.tsx @@ -640,9 +640,18 @@ const FeedbackButtons: FC = () => { 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)); + if (next) { + void db.messages + .update(messageId, { feedback: next }) + .catch((err) => console.error("Failed to save feedback:", err)); + } else { + // Dexie ignores undefined values in update(), so use modify+delete + void db.messages + .where("id") + .equals(messageId) + .modify((msg) => { delete msg.feedback; }) + .catch((err) => console.error("Failed to clear feedback:", err)); + } } return next; }); diff --git a/studio/frontend/src/features/chat/chat-page.tsx b/studio/frontend/src/features/chat/chat-page.tsx index 5c2da42dfb..b0ed17581b 100644 --- a/studio/frontend/src/features/chat/chat-page.tsx +++ b/studio/frontend/src/features/chat/chat-page.tsx @@ -530,6 +530,7 @@ export function ChatPage(): ReactElement { const handleNewThread = useCallback( () => { useChatRuntimeStore.getState().setActiveThreadId(null); + useArtifactStore.getState().clearArtifacts(); setView({ mode: "single", newThreadNonce: crypto.randomUUID() }); }, [], @@ -592,6 +593,7 @@ export function ChatPage(): ReactElement { const handleThreadSelect = useCallback( (nextView: ChatView) => { + useArtifactStore.getState().clearArtifacts(); setView(nextView); }, [], diff --git a/studio/frontend/src/features/chat/components/artifact-panel.tsx b/studio/frontend/src/features/chat/components/artifact-panel.tsx index 4f90ed9cf6..9e47507dc4 100644 --- a/studio/frontend/src/features/chat/components/artifact-panel.tsx +++ b/studio/frontend/src/features/chat/components/artifact-panel.tsx @@ -64,22 +64,30 @@ export const ArtifactPanel: FC = () => { const [copied, setCopied] = useState(false); const [localValue, setLocalValue] = useState(""); - const resetRef = useRef>(); - const textareaRef = useRef(null); + const resetRef = useRef | null>(null); + const textareaRef = useRef(null); const active = artifacts.find((a) => a.id === activeId) ?? artifacts[0]; + const viewedContent = active + ? (active.history[active.activeVersion] ?? active.content) + : ""; - // Sync local editor value when switching between artifacts + // Sync local editor value when switching artifacts or versions useEffect(() => { - if (active) setLocalValue(active.content); - // Only reset on tab switch, not on content updates (which would clobber edits) - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [active?.id]); + if (active) setLocalValue(viewedContent); + }, [active?.id, active?.activeVersion, viewedContent]); + + // Cleanup copy timer on unmount + useEffect(() => { + return () => { + if (resetRef.current) clearTimeout(resetRef.current); + }; + }, []); if (!panelOpen || artifacts.length === 0 || !active) return null; const handleCopy = () => { - if (copyToClipboard(active.content)) { + if (copyToClipboard(localValue)) { setCopied(true); if (resetRef.current) clearTimeout(resetRef.current); resetRef.current = setTimeout(() => setCopied(false), COPY_RESET_MS); @@ -94,7 +102,7 @@ export const ArtifactPanel: FC = () => { : active.language ? `.${active.language}` : ".txt"; - downloadTextFile(`${active.title}${ext}`, active.content); + downloadTextFile(`${active.title}${ext}`, localValue); }; const canPrev = active.activeVersion > 0; @@ -164,7 +172,7 @@ export const ArtifactPanel: FC = () => { value={localValue} onChange={(e) => setLocalValue(e.target.value)} onBlur={() => { - if (localValue !== active.content) { + if (localValue !== viewedContent) { updateContent(active.id, localValue); } }} diff --git a/studio/frontend/src/features/chat/db.ts b/studio/frontend/src/features/chat/db.ts index 55cffcd34c..02cbfc14c5 100644 --- a/studio/frontend/src/features/chat/db.ts +++ b/studio/frontend/src/features/chat/db.ts @@ -54,32 +54,31 @@ db.version(4) memory: "id, createdAt", }) .upgrade(async (tx) => { - // Backfill searchText from first user message in each thread + // Backfill searchText from first user message in each thread. + // Process sequentially to avoid IndexedDB transaction auto-commit. const threads = await tx.table("threads").toArray(); - await Promise.all( - threads.map(async (thread) => { - const msgs = await tx - .table("messages") - .where("threadId") - .equals(thread.id) - .toArray(); - const firstUser = msgs - .sort((a: MessageRecord, b: MessageRecord) => a.createdAt - b.createdAt) - .find((m: MessageRecord) => m.role === "user"); - if (!firstUser) return; - const textParts = Array.isArray(firstUser.content) - ? firstUser.content - .filter((p: { type: string }) => p.type === "text") - .map((p: { text: string }) => p.text) - .join(" ") - : ""; - if (textParts.trim()) { - await tx - .table("threads") - .update(thread.id, { searchText: textParts.slice(0, 500) }); - } - }), - ); + for (const thread of threads) { + const msgs = await tx + .table("messages") + .where("threadId") + .equals(thread.id) + .sortBy("createdAt"); + const firstUser = msgs.find( + (m: MessageRecord) => m.role === "user", + ); + if (!firstUser) continue; + const textParts = Array.isArray(firstUser.content) + ? firstUser.content + .filter((p: { type: string }) => p.type === "text") + .map((p: { text: string }) => p.text) + .join(" ") + : ""; + if (textParts.trim()) { + await tx + .table("threads") + .update(thread.id, { searchText: textParts.slice(0, 500) }); + } + } }); export { db }; diff --git a/studio/frontend/src/features/chat/lib/thread-export.ts b/studio/frontend/src/features/chat/lib/thread-export.ts index 102e9046f2..f9491e3e9f 100644 --- a/studio/frontend/src/features/chat/lib/thread-export.ts +++ b/studio/frontend/src/features/chat/lib/thread-export.ts @@ -45,7 +45,7 @@ export async function exportAsMarkdown(threadId: string): Promise { } downloadTextFile( - `${sanitizeFilename(thread.title)}.md`, + buildExportFilename(thread, "md"), lines.join("\n"), "text/markdown", ); @@ -63,7 +63,7 @@ export async function exportAsJSON(threadId: string): Promise { }; downloadTextFile( - `${sanitizeFilename(thread.title)}.json`, + buildExportFilename(thread, "json"), JSON.stringify(payload, null, 2), "application/json", ); @@ -82,7 +82,7 @@ export async function exportAsJSONL(threadId: string): Promise { const line = JSON.stringify({ messages: chatMessages }); downloadTextFile( - `${sanitizeFilename(thread.title)}.jsonl`, + buildExportFilename(thread, "jsonl"), line + "\n", "application/x-ndjson", ); @@ -90,12 +90,23 @@ export async function exportAsJSONL(threadId: string): Promise { function sanitizeFilename(name: string): string { return name - .replace(/[^a-zA-Z0-9_\- ]/g, "") + .replace(/[^\p{L}\p{N}_\- ]/gu, "") .replace(/\s+/g, "_") .slice(0, 80) || "chat_export"; } +function buildExportFilename( + thread: ThreadRecord, + ext: "md" | "json" | "jsonl", +): string { + const base = sanitizeFilename(thread.title); + const suffix = thread.pairId + ? `_${sanitizeFilename(thread.modelId || thread.modelType || "compare")}` + : ""; + return `${base}${suffix}.${ext}`; +} + export async function getExportThreadIds( threadOrPairId: string, type: "single" | "compare", diff --git a/studio/frontend/src/features/chat/thread-sidebar.tsx b/studio/frontend/src/features/chat/thread-sidebar.tsx index 033293cb92..4521918094 100644 --- a/studio/frontend/src/features/chat/thread-sidebar.tsx +++ b/studio/frontend/src/features/chat/thread-sidebar.tsx @@ -135,11 +135,13 @@ export function ThreadSidebar({ // Filter items by matching title or searchText from underlying threads return items.filter((item) => { if (item.title.toLowerCase().includes(q)) return true; - // Check searchText on the underlying thread records - const thread = (allThreads ?? []).find( - (t) => t.id === item.id || t.pairId === item.id, + // Check searchText on all underlying thread records (both sides of compare pairs) + const relatedThreads = (allThreads ?? []).filter((t) => + item.type === "single" ? t.id === item.id : t.pairId === item.id, + ); + return relatedThreads.some( + (t) => t.searchText?.toLowerCase().includes(q), ); - return thread?.searchText?.toLowerCase().includes(q) ?? false; }); }, [items, debouncedQuery, allThreads]); @@ -200,14 +202,22 @@ export function ThreadSidebar({ } async function handleMoveToFolder(item: SidebarItem, folderId: string | undefined) { - const newFolderId = folderId || undefined; if (item.type === "single") { - await db.threads.update(item.id, { folderId: newFolderId }); + await db.threads.where("id").equals(item.id).modify((thread) => { + if (folderId) { + thread.folderId = folderId; + } else { + delete thread.folderId; + } + }); } else { - const paired = await db.threads.where("pairId").equals(item.id).toArray(); - for (const t of paired) { - await db.threads.update(t.id, { folderId: newFolderId }); - } + await db.threads.where("pairId").equals(item.id).modify((thread) => { + if (folderId) { + thread.folderId = folderId; + } else { + delete thread.folderId; + } + }); } } diff --git a/studio/frontend/src/lib/download.ts b/studio/frontend/src/lib/download.ts index fce2e371d3..4cad9db032 100644 --- a/studio/frontend/src/lib/download.ts +++ b/studio/frontend/src/lib/download.ts @@ -8,11 +8,14 @@ export function downloadTextFile( ): void { const blob = new Blob([content], { type: mimeType }); const url = URL.createObjectURL(blob); - const a = document.createElement("a"); - a.href = url; - a.download = filename; - document.body.appendChild(a); - a.click(); - document.body.removeChild(a); - setTimeout(() => URL.revokeObjectURL(url), 100); + try { + const a = document.createElement("a"); + a.href = url; + a.download = filename; + document.body.appendChild(a); + a.click(); + document.body.removeChild(a); + } finally { + setTimeout(() => URL.revokeObjectURL(url), 1000); + } }