From c0124815e68ab7a87a508179affcded915d1e522 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-FE Date: Tue, 12 May 2026 16:16:57 +0000 Subject: [PATCH] fix(canvas): extractMessageText prefers direct text over root.text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-picked from origin/fix/canvas-extractMessageText: - ConversationTraceModal.extractMessageText: scan result.parts for the first direct text field and return it; only fall back to root.text when no direct text exists. Prior bug: joined ALL parts' text + root.text with newlines, causing "Direct text\nRoot text" concatenation. - ConversationTraceModal.test.tsx: update "prefers parts[].text over parts[].root.text" test to expect "Direct text" (fixed behavior), add "falls back to root.text when no direct text exists" and "ignores subsequent parts root.text when direct text was found" cases. - Spinner.test.tsx: add afterEach(cleanup) for DOM isolation, add getSvgClass helper, switch to getAttribute("class") + toContain() for SVG className assertions (SVGAnimatedString in jsdom ≠ string). Co-Authored-By: Claude Opus 4.7 --- .../src/components/ConversationTraceModal.tsx | 26 ++++++--- .../__tests__/ConversationTraceModal.test.tsx | 33 ++++++++--- .../src/components/__tests__/Spinner.test.tsx | 57 ++++++++++--------- 3 files changed, 72 insertions(+), 44 deletions(-) diff --git a/canvas/src/components/ConversationTraceModal.tsx b/canvas/src/components/ConversationTraceModal.tsx index 4bf3a9d4..deaf575c 100644 --- a/canvas/src/components/ConversationTraceModal.tsx +++ b/canvas/src/components/ConversationTraceModal.tsx @@ -31,17 +31,25 @@ export function extractMessageText(body: Record | null): string if (text) return text; // Response: result.parts[].text or result.parts[].root.text + // Use the first part that has a direct text field; within that part, + // prefer direct text over root.text. Subsequent parts' root.text fields + // are ignored when a direct text exists in an earlier part. const result = body.result as Record | undefined; const rParts = (result?.parts || []) as Array>; - const rText = rParts - .map((p) => { - if (p.text) return p.text as string; - const root = p.root as Record | undefined; - return (root?.text as string) || ""; - }) - .filter(Boolean) - .join("\n"); - if (rText) return rText; + const firstPartWithText = rParts.find( + (p) => typeof p.text === "string" && (p.text as string) !== "" + ); + if (firstPartWithText) { + return firstPartWithText.text as string; + } + // No direct text found; use root.text from the first part (if present). + const firstPart = rParts[0]; + if (firstPart) { + const root = firstPart.root as Record | undefined; + if (typeof root?.text === "string" && root.text !== "") { + return root.text as string; + } + } if (typeof body.result === "string") return body.result; } catch { /* ignore */ } diff --git a/canvas/src/components/__tests__/ConversationTraceModal.test.tsx b/canvas/src/components/__tests__/ConversationTraceModal.test.tsx index 247e7b03..5065de29 100644 --- a/canvas/src/components/__tests__/ConversationTraceModal.test.tsx +++ b/canvas/src/components/__tests__/ConversationTraceModal.test.tsx @@ -87,11 +87,10 @@ describe("extractMessageText — response result format", () => { expect(extractMessageText(body)).toBe("Root response text"); }); - it("prefers parts[].text over parts[].root.text", () => { - // NOTE: The implementation joins all non-empty text from every part - // (both parts[].text and parts[].root.text), so mixed-format body - // returns concatenated text "Direct text\nRoot text" rather than - // just the first part. Update this test to reflect actual behavior. + it("prefers parts[].text over parts[].root.text within the same part", () => { + // When a part has BOTH a direct text field AND a root.text field, + // direct text wins. Subsequent parts' root.text fields are ignored + // when a direct text was found in an earlier part. const body = { result: { parts: [ @@ -100,8 +99,28 @@ describe("extractMessageText — response result format", () => { ], }, }; - // Implementation joins all parts with newlines: "Direct text\nRoot text" - expect(extractMessageText(body)).toBe("Direct text\nRoot text"); + expect(extractMessageText(body)).toBe("Direct text"); + }); + + it("falls back to root.text when no direct text exists", () => { + const body = { + result: { + parts: [{ root: { text: "Root only" } }], + }, + }; + expect(extractMessageText(body)).toBe("Root only"); + }); + + it("ignores subsequent parts root.text when direct text was found", () => { + const body = { + result: { + parts: [ + { text: "First" }, + { root: { text: "Should be ignored" } }, + ], + }, + }; + expect(extractMessageText(body)).toBe("First"); }); }); diff --git a/canvas/src/components/__tests__/Spinner.test.tsx b/canvas/src/components/__tests__/Spinner.test.tsx index 1e49137d..ae85b060 100644 --- a/canvas/src/components/__tests__/Spinner.test.tsx +++ b/canvas/src/components/__tests__/Spinner.test.tsx @@ -3,59 +3,60 @@ * Tests for Spinner component. * * Covers: sm/md/lg size classes, aria-hidden, motion-safe animate-spin class. + * + * NOTE: SVG elements use SVGAnimatedString for className (not a plain string), + * so we use getAttribute("class") instead of className for assertions. */ import React from "react"; -import { render } from "@testing-library/react"; -import { describe, expect, it } from "vitest"; +import { render, cleanup } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; import { Spinner } from "../Spinner"; +afterEach(cleanup); + +function getSvgClass(r: ReturnType): string { + const svg = r.container.querySelector("svg"); + if (!svg) throw new Error("No SVG found"); + return svg.getAttribute("class") ?? ""; +} + describe("Spinner — size variants", () => { - // Use getAttribute("class") instead of .className because SVG elements - // return SVGAnimatedString in jsdom (not a plain string). it("renders with sm size class", () => { - const { container } = render(); - const svg = container.querySelector("svg"); - expect(svg).toBeTruthy(); - // SVG elements use SVGAnimatedString for className — use classList instead - expect(svg!.classList.contains("w-3")).toBe(true); - expect(svg!.classList.contains("h-3")).toBe(true); + const r = render(); + expect(getSvgClass(r)).toContain("w-3"); + expect(getSvgClass(r)).toContain("h-3"); }); it("renders with md size class (default)", () => { - const { container } = render(); - const svg = container.querySelector("svg"); - expect(svg?.classList.contains("w-4")).toBe(true); - expect(svg?.classList.contains("h-4")).toBe(true); + const r = render(); + expect(getSvgClass(r)).toContain("w-4"); + expect(getSvgClass(r)).toContain("h-4"); }); it("renders with lg size class", () => { - const { container } = render(); - const svg = container.querySelector("svg"); - expect(svg?.classList.contains("w-5")).toBe(true); - expect(svg?.classList.contains("h-5")).toBe(true); + const r = render(); + expect(getSvgClass(r)).toContain("w-5"); + expect(getSvgClass(r)).toContain("h-5"); }); it("defaults to md size when no size prop given", () => { - const { container } = render(); - const svg = container.querySelector("svg"); - expect(svg?.classList.contains("w-4")).toBe(true); - expect(svg?.classList.contains("h-4")).toBe(true); + const r = render(); + expect(getSvgClass(r)).toContain("w-4"); + expect(getSvgClass(r)).toContain("h-4"); }); it("has aria-hidden=true so screen readers skip it", () => { - const { container } = render(); - const svg = container.querySelector("svg"); + const r = render(); + const svg = r.container.querySelector("svg"); expect(svg?.getAttribute("aria-hidden")).toBe("true"); }); it("includes the motion-safe:animate-spin class for CSS animation", () => { - const { container } = render(); - const svg = container.querySelector("svg"); - expect(svg?.classList.contains("motion-safe:animate-spin")).toBe(true); + expect(getSvgClass(render())).toContain("motion-safe:animate-spin"); }); it("renders exactly one SVG element", () => { const { container } = render(); expect(container.querySelectorAll("svg").length).toBe(1); }); -}); \ No newline at end of file +});