fix(canvas): extractMessageText prefers direct text over root.text
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 11s
Harness Replays / detect-changes (pull_request) Successful in 17s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 15s
CI / Detect changes (pull_request) Successful in 34s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 37s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 36s
qa-review / approved (pull_request) Failing after 17s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 46s
security-review / approved (pull_request) Failing after 15s
E2E API Smoke Test / detect-changes (pull_request) Successful in 48s
Harness Replays / Harness Replays (pull_request) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 6s
CI / Platform (Go) (pull_request) Successful in 7s
CI / Python Lint & Test (pull_request) Successful in 7s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 8s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 10s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 10s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4
sop-checklist-gate / gate (pull_request) Successful in 14s
sop-tier-check / tier-check (pull_request) Successful in 13s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m21s
gate-check-v3 / gate-check (pull_request) Failing after 24s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 7m37s
CI / Canvas (Next.js) (pull_request) Successful in 10m57s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 3s
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 11s
Harness Replays / detect-changes (pull_request) Successful in 17s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 15s
CI / Detect changes (pull_request) Successful in 34s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 37s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 36s
qa-review / approved (pull_request) Failing after 17s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 46s
security-review / approved (pull_request) Failing after 15s
E2E API Smoke Test / detect-changes (pull_request) Successful in 48s
Harness Replays / Harness Replays (pull_request) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 6s
CI / Platform (Go) (pull_request) Successful in 7s
CI / Python Lint & Test (pull_request) Successful in 7s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 8s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 10s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 10s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4
sop-checklist-gate / gate (pull_request) Successful in 14s
sop-tier-check / tier-check (pull_request) Successful in 13s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m21s
gate-check-v3 / gate-check (pull_request) Failing after 24s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 7m37s
CI / Canvas (Next.js) (pull_request) Successful in 10m57s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 3s
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 <noreply@anthropic.com>
This commit is contained in:
parent
4deefb5b96
commit
d9e874dbeb
@ -31,17 +31,25 @@ export function extractMessageText(body: Record<string, unknown> | 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<string, unknown> | undefined;
|
||||
const rParts = (result?.parts || []) as Array<Record<string, unknown>>;
|
||||
const rText = rParts
|
||||
.map((p) => {
|
||||
if (p.text) return p.text as string;
|
||||
const root = p.root as Record<string, unknown> | 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<string, unknown> | undefined;
|
||||
if (typeof root?.text === "string" && root.text !== "") {
|
||||
return root.text as string;
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof body.result === "string") return body.result;
|
||||
} catch { /* ignore */ }
|
||||
|
||||
@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@ -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<typeof render>): 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(<Spinner size="sm" />);
|
||||
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(<Spinner size="sm" />);
|
||||
expect(getSvgClass(r)).toContain("w-3");
|
||||
expect(getSvgClass(r)).toContain("h-3");
|
||||
});
|
||||
|
||||
it("renders with md size class (default)", () => {
|
||||
const { container } = render(<Spinner size="md" />);
|
||||
const svg = container.querySelector("svg");
|
||||
expect(svg?.classList.contains("w-4")).toBe(true);
|
||||
expect(svg?.classList.contains("h-4")).toBe(true);
|
||||
const r = render(<Spinner size="md" />);
|
||||
expect(getSvgClass(r)).toContain("w-4");
|
||||
expect(getSvgClass(r)).toContain("h-4");
|
||||
});
|
||||
|
||||
it("renders with lg size class", () => {
|
||||
const { container } = render(<Spinner size="lg" />);
|
||||
const svg = container.querySelector("svg");
|
||||
expect(svg?.classList.contains("w-5")).toBe(true);
|
||||
expect(svg?.classList.contains("h-5")).toBe(true);
|
||||
const r = render(<Spinner size="lg" />);
|
||||
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(<Spinner />);
|
||||
const svg = container.querySelector("svg");
|
||||
expect(svg?.classList.contains("w-4")).toBe(true);
|
||||
expect(svg?.classList.contains("h-4")).toBe(true);
|
||||
const r = render(<Spinner />);
|
||||
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(<Spinner />);
|
||||
const svg = container.querySelector("svg");
|
||||
const r = render(<Spinner />);
|
||||
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(<Spinner />);
|
||||
const svg = container.querySelector("svg");
|
||||
expect(svg?.classList.contains("motion-safe:animate-spin")).toBe(true);
|
||||
expect(getSvgClass(render(<Spinner />))).toContain("motion-safe:animate-spin");
|
||||
});
|
||||
|
||||
it("renders exactly one SVG element", () => {
|
||||
const { container } = render(<Spinner />);
|
||||
expect(container.querySelectorAll("svg").length).toBe(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user