From 1aef0eca5117b852ccc0c1affac8e3febc910a96 Mon Sep 17 00:00:00 2001 From: wasimysaid Date: Mon, 25 May 2026 19:15:22 +0200 Subject: [PATCH] Studio: fix chat artifact review regressions --- .../components/assistant-ui/markdown-text.tsx | 9 +-- .../assistant-ui/tool-ui-render-html.tsx | 26 +++++-- .../src/features/chat/api/chat-adapter.ts | 30 +++++--- .../features/chat/artifacts/artifact-card.tsx | 5 +- .../chat/artifacts/artifact-surface.tsx | 74 ++++++++++++++++++- .../features/chat/artifacts/html-frame.tsx | 3 + .../src/features/chat/artifacts/store.ts | 4 +- .../src/features/chat/artifacts/types.ts | 13 ++-- 8 files changed, 127 insertions(+), 37 deletions(-) diff --git a/studio/frontend/src/components/assistant-ui/markdown-text.tsx b/studio/frontend/src/components/assistant-ui/markdown-text.tsx index c43e9383b1..35c5c16473 100644 --- a/studio/frontend/src/components/assistant-ui/markdown-text.tsx +++ b/studio/frontend/src/components/assistant-ui/markdown-text.tsx @@ -10,12 +10,12 @@ import { openLink } from "@/lib/open-link"; import { INTERNAL, useMessagePartText } from "@assistant-ui/react"; import { Copy01Icon, Tick02Icon } from "@hugeicons/core-free-icons"; import { HugeiconsIcon } from "@hugeicons/react"; -import { createCodePlugin } from "./code-plugin"; import { createMathPlugin } from "@streamdown/math"; import { mermaid } from "@streamdown/mermaid"; import { DownloadIcon } from "lucide-react"; import { useEffect, useMemo, useRef, useState } from "react"; import { Block, type BlockProps, Streamdown } from "streamdown"; +import { createCodePlugin } from "./code-plugin"; import "katex/dist/katex.min.css"; import { AudioPlayer } from "./audio-player"; import { unslothDarkTheme, unslothLightTheme } from "./code-themes"; @@ -307,12 +307,7 @@ function StreamdownBlock(props: BlockProps) { !props.isIncomplete && isHtmlFence(codeFence) ? codeFence.source : null; if (htmlSource) { return ( - + ); } diff --git a/studio/frontend/src/components/assistant-ui/tool-ui-render-html.tsx b/studio/frontend/src/components/assistant-ui/tool-ui-render-html.tsx index d951d8a6ba..7e125183af 100644 --- a/studio/frontend/src/components/assistant-ui/tool-ui-render-html.tsx +++ b/studio/frontend/src/components/assistant-ui/tool-ui-render-html.tsx @@ -21,15 +21,28 @@ interface RenderHtmlArgs { title?: string; } +function formatToolResult(result: unknown): string { + if (typeof result === "string") return result; + if (result == null) return ""; + try { + return JSON.stringify(result, null, 2); + } catch { + return String(result); + } +} + const RenderHtmlToolUIImpl: ToolCallMessagePartComponent = ({ args, + result, status, toolCallId, }) => { const parsedArgs = (args as RenderHtmlArgs) ?? {}; const code = typeof parsedArgs.code === "string" ? parsedArgs.code : ""; + const hasCode = code.trim().length > 0; const title = typeof parsedArgs.title === "string" ? parsedArgs.title : "HTML artifact"; + const resultText = formatToolResult(result); const isRunning = status?.type === "running"; const hasText = useAuiState(({ message }) => message.content.some( @@ -42,11 +55,10 @@ const RenderHtmlToolUIImpl: ToolCallMessagePartComponent = ({ const [open, setOpen] = useState(true); useEffect(() => { - const nextOpen = isRunning || code ? true : hasText ? false : null; - if (nextOpen == null) return; + const nextOpen = isRunning ? true : !hasText; const timeoutId = window.setTimeout(() => setOpen(nextOpen), 0); return () => window.clearTimeout(timeoutId); - }, [code, hasText, isRunning]); + }, [hasText, isRunning]); return ( @@ -56,12 +68,12 @@ const RenderHtmlToolUIImpl: ToolCallMessagePartComponent = ({ icon={FileTextIcon} /> - {isRunning && !code ? ( + {isRunning && !hasCode ? (
Waiting for artifact source…
- ) : code ? ( + ) : hasCode ? ( ) : ( -

- No HTML artifact source was provided. +

+ {resultText || "No HTML artifact source was provided."}

)} diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index 4e6e9663c4..95cac00f22 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-only // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 -import { getAuthToken } from "@/features/auth/session"; +import { getAuthToken } from "@/features/auth"; import { apiUrl } from "@/lib/api-base"; import { toast } from "@/lib/toast"; import type { MessageTiming, ToolCallMessagePart } from "@assistant-ui/core"; @@ -37,12 +37,12 @@ import type { OpenAIMessageContent, } from "../types/api"; import type { ChatModelSummary } from "../types/runtime"; -import { getImageInputUnavailableReason } from "../utils/image-input-support"; import { getStoredChatThread, listStoredChatThreads, updateStoredChatThread, } from "../utils/chat-history-storage"; +import { getImageInputUnavailableReason } from "../utils/image-input-support"; import { hasClosedThinkTag, parseAssistantContent, @@ -994,11 +994,19 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { targetMessages.unshift({ role: "system", content: text }); } - // Keep render_html local-only for now. External providers already have - // provider-specific built-in tool translation, but no generic custom-tool - // round-trip loop; they get the fenced-html artifact fallback instead. + const imageBase64 = findLatestUserImageBase64(messages); + const audioBase64 = findLatestUserAudioBase64(messages); + const hasOutboundImage = Boolean(imageBase64); + + // Keep render_html local-only and mirror the backend image-turn gate. + // GGUF/safetensors disable tool execution when an image is present, so + // image turns receive the fenced-html artifact fallback instead of being + // prompted to call a tool the backend will not expose. const renderHtmlToolEnabledForThisTurn = Boolean( - !isExternalRequest && supportsTools && artifactsEnabled, + !isExternalRequest && + supportsTools && + artifactsEnabled && + !hasOutboundImage, ); const artifactInstruction = artifactsEnabled ? renderHtmlToolEnabledForThisTurn @@ -1011,8 +1019,6 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { : disabledToolGuard; addSystemInstruction(outboundMessages, effectiveDisabledToolGuard); addSystemInstruction(outboundMessages, artifactInstruction); - const imageBase64 = findLatestUserImageBase64(messages); - const audioBase64 = findLatestUserAudioBase64(messages); // Block when ANY image is in the outbound payload (current or // prior turns) and the loaded model can't process images. Keeps @@ -1542,13 +1548,17 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { ? { preserve_thinking: preserveThinking } : {}), ...(supportsTools && - (toolsEnabled || codeToolsEnabled || artifactsEnabled) + (toolsEnabled || + codeToolsEnabled || + renderHtmlToolEnabledForThisTurn) ? { enable_tools: true, enabled_tools: [ ...(toolsEnabled ? ["web_search"] : []), ...(codeToolsEnabled ? ["python", "terminal"] : []), - ...(artifactsEnabled ? ["render_html"] : []), + ...(renderHtmlToolEnabledForThisTurn + ? ["render_html"] + : []), ], auto_heal_tool_calls: useChatRuntimeStore.getState().autoHealToolCalls, diff --git a/studio/frontend/src/features/chat/artifacts/artifact-card.tsx b/studio/frontend/src/features/chat/artifacts/artifact-card.tsx index 3de7da1a2e..1a6a088931 100644 --- a/studio/frontend/src/features/chat/artifacts/artifact-card.tsx +++ b/studio/frontend/src/features/chat/artifacts/artifact-card.tsx @@ -5,7 +5,6 @@ import { copyToClipboard } from "@/lib/copy-to-clipboard"; import { cn } from "@/lib/utils"; -import { useChatRuntimeStore } from "../stores/chat-runtime-store"; import { useAuiState } from "@assistant-ui/react"; import { CheckIcon, @@ -15,6 +14,7 @@ import { FileTextIcon, } from "lucide-react"; import { useEffect, useMemo, useRef, useState } from "react"; +import { useChatRuntimeStore } from "../stores/chat-runtime-store"; import { ArtifactHtmlFrame } from "./html-frame"; import { useChatArtifactsStore } from "./store"; import { @@ -133,6 +133,7 @@ export function ArtifactCard({ type="button" className="flex size-8 items-center justify-center rounded-lg text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" title="Open artifact" + aria-label="Open artifact" onClick={() => openArtifact(artifact, { surface })} > @@ -141,6 +142,7 @@ export function ArtifactCard({ type="button" className="flex size-8 items-center justify-center rounded-lg text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" title="Copy HTML" + aria-label="Copy artifact HTML" onClick={async () => { if (await copyToClipboard(artifact.code)) showCopied(); }} @@ -155,6 +157,7 @@ export function ArtifactCard({ type="button" className="flex size-8 items-center justify-center rounded-lg text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" title="Download HTML" + aria-label="Download artifact HTML" onClick={() => downloadTextFile(filename, artifact.code)} > diff --git a/studio/frontend/src/features/chat/artifacts/artifact-surface.tsx b/studio/frontend/src/features/chat/artifacts/artifact-surface.tsx index 476d2521b9..f5fce60c08 100644 --- a/studio/frontend/src/features/chat/artifacts/artifact-surface.tsx +++ b/studio/frontend/src/features/chat/artifacts/artifact-surface.tsx @@ -14,12 +14,25 @@ import { Maximize2Icon, XIcon, } from "lucide-react"; -import { useEffect, useRef, useState } from "react"; +import { type KeyboardEvent, useEffect, useRef, useState } from "react"; import { ArtifactHtmlFrame, type ArtifactViewMode } from "./html-frame"; import type { ChatArtifact } from "./types"; import { getArtifactFilename } from "./types"; const COPY_RESET_MS = 2000; +const FOCUSABLE_SELECTOR = + 'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])'; + +function getFocusableElements(container: HTMLElement): HTMLElement[] { + return Array.from( + container.querySelectorAll(FOCUSABLE_SELECTOR), + ).filter( + (element) => + !element.hasAttribute("disabled") && + element.getAttribute("aria-hidden") !== "true" && + element.tabIndex !== -1, + ); +} function downloadTextFile(filename: string, text: string): void { const blob = new Blob([text], { type: "text/html;charset=utf-8" }); @@ -47,6 +60,8 @@ export function ArtifactSurface({ const [viewMode, setViewMode] = useState("preview"); const [copied, setCopied] = useState(false); const copyResetRef = useRef | null>(null); + const surfaceRef = useRef(null); + const previousFocusRef = useRef(null); const filename = getArtifactFilename(artifact); useEffect(() => { @@ -55,6 +70,26 @@ export function ArtifactSurface({ }; }, []); + useEffect(() => { + if (variant !== "overlay") return; + previousFocusRef.current = document.activeElement; + const timeoutId = window.setTimeout(() => { + const surface = surfaceRef.current; + if (!surface) return; + const firstFocusable = getFocusableElements(surface)[0]; + if (firstFocusable) { + firstFocusable.focus(); + } else { + surface.focus(); + } + }, 0); + return () => { + window.clearTimeout(timeoutId); + const previousFocus = previousFocusRef.current; + if (previousFocus instanceof HTMLElement) previousFocus.focus(); + }; + }, [variant]); + const handleCopy = async () => { if (!(await copyToClipboard(artifact.code))) return; setCopied(true); @@ -65,8 +100,38 @@ export function ArtifactSurface({ }, COPY_RESET_MS); }; + const handleDialogKeyDown = (event: KeyboardEvent) => { + if (variant !== "overlay") return; + if (event.key === "Escape") { + event.preventDefault(); + onClose(); + return; + } + if (event.key !== "Tab") return; + const focusable = getFocusableElements(event.currentTarget); + if (focusable.length === 0) { + event.preventDefault(); + event.currentTarget.focus(); + return; + } + const first = focusable[0]; + const last = focusable[focusable.length - 1]; + if (event.shiftKey && document.activeElement === first) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && document.activeElement === last) { + event.preventDefault(); + first.focus(); + } + }; + const content = (
) : ( - 
+          
             {artifact.code}
           

         )}
@@ -179,8 +244,9 @@ export function ArtifactSurface({
     return (
       
 {
+          if (event.target === event.currentTarget) onClose();
+        }}
       >
         {content}
       

diff --git a/studio/frontend/src/features/chat/artifacts/html-frame.tsx b/studio/frontend/src/features/chat/artifacts/html-frame.tsx
index 26a9924467..f3814e7b4e 100644
--- a/studio/frontend/src/features/chat/artifacts/html-frame.tsx
+++ b/studio/frontend/src/features/chat/artifacts/html-frame.tsx
@@ -24,6 +24,9 @@ export function buildArtifactSrcDoc(code: string): string {
   return `${code}\n${resizeScript}`;
 }
 
+// Preview iframes intentionally omit allow-downloads: generated artifacts can
+// offer their own UI, but downloads must go through Studio's explicit
+// copy/download controls outside the no-same-origin sandbox.
 export function ArtifactHtmlFrame({
   code,
   title = "HTML artifact preview",
diff --git a/studio/frontend/src/features/chat/artifacts/store.ts b/studio/frontend/src/features/chat/artifacts/store.ts
index d75f79968b..764b0136bc 100644
--- a/studio/frontend/src/features/chat/artifacts/store.ts
+++ b/studio/frontend/src/features/chat/artifacts/store.ts
@@ -24,13 +24,13 @@ export const useChatArtifactsStore = create((set) => ({
   openArtifact: (artifact, options) =>
     set((state) => ({
       artifactsById: {
-        ...state.artifactsById,
         [artifact.id]: artifact,
       },
       selectedArtifactId: artifact.id,
       surface: options?.surface ?? state.surface,
     })),
-  closeArtifactSurface: () => set({ selectedArtifactId: null }),
+  closeArtifactSurface: () =>
+    set({ artifactsById: {}, selectedArtifactId: null }),
   clearArtifactsForThread: (threadId) =>
     set((state) => {
       if (!threadId) return state;
diff --git a/studio/frontend/src/features/chat/artifacts/types.ts b/studio/frontend/src/features/chat/artifacts/types.ts
index bfe75d83e4..4248becb40 100644
--- a/studio/frontend/src/features/chat/artifacts/types.ts
+++ b/studio/frontend/src/features/chat/artifacts/types.ts
@@ -43,12 +43,13 @@ export function createArtifactId(input: ChatArtifactInput): string {
   const threadSegment = input.threadId || "no-thread";
   const sourceId =
     input.sourceToolCallId || input.sourceMessageId || "transient";
-  return [
-    input.source,
-    threadSegment,
-    sourceId,
-    hashArtifactCode(input.code),
-  ].join(":");
+  const parts = [input.source, threadSegment, sourceId];
+
+  if (input.source !== "tool" || !input.sourceToolCallId) {
+    parts.push(hashArtifactCode(input.code));
+  }
+
+  return parts.join(":");
 }
 
 export function createChatArtifact(input: ChatArtifactInput): ChatArtifact {