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
2026-03-31 11:28:04 +00:00 · 2026-03-31 11:28:04 +00:00 · 007ffd2569
commit 007ffd2569
parent 9d61147d65
7 changed files with 101 additions and 59 deletions
--- 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;
      });
--- 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);
    },
    [],
--- 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<ReturnType<typeof setTimeout>>();
-  const textareaRef = useRef<HTMLTextAreaElement>(null);
+  const resetRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const textareaRef = useRef<HTMLTextAreaElement | null>(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);
            }
          }}
--- 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 };
--- 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<void> {
  }

  downloadTextFile(
-    `${sanitizeFilename(thread.title)}.md`,
+    buildExportFilename(thread, "md"),
    lines.join("\n"),
    "text/markdown",
  );
@ -63,7 +63,7 @@ export async function exportAsJSON(threadId: string): Promise<void> {
  };

  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<void> {

  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<void> {

 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",
--- 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;
+        }
+      });
    }
  }

--- 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);
+  }
 }