diff --git a/narratives/ui/src/components/card_response.tsx b/narratives/ui/src/components/card_response.tsx index 1b9ea8ddd..c37abcc34 100644 --- a/narratives/ui/src/components/card_response.tsx +++ b/narratives/ui/src/components/card_response.tsx @@ -42,14 +42,13 @@ import { textOf, type MarkdownNode, type TableAnalysis, -} from "../utils/markdown_table"; -import { rehypeStreamWords } from "../utils/rehype_stream_words"; -import type { ProvenanceItem } from "../hooks/use_sse_chat"; +} from "@/src/utils/markdown_table"; +import { rehypeStreamWords } from "@/src/utils/rehype_stream_words"; +import type { ProvenanceItem } from "@/src/hooks/use_sse_chat"; interface ResponseCardProps { title: string; body: string; - streaming?: boolean; // The answer's provenance list, so an inline [n] chip can name the source // it points at rather than showing a bare number. Position n is chip [n] -- // the same order the synthesis prompt numbered and CitationsList renders. @@ -61,6 +60,9 @@ interface ResponseCardProps { // When set, the content animates in over this many milliseconds. When // unset, it appears immediately. streamMs?: number; + // Delay before the first word animates, in milliseconds, so a body that + // follows another in one reveal starts where the previous one ends. + streamStartMs?: number; // Whether the animation is still running. Once it ends the animation class // is dropped, so a copy of the DOM (the PDF export) does not replay it. revealing?: boolean; @@ -72,10 +74,10 @@ const LINK_COLOR = "var(--color-theme-primary)"; export function ResponseCard({ title, body, - streaming, sources, bare = false, streamMs, + streamStartMs, revealing = streamMs !== undefined, }: ResponseCardProps) { const markdown = ( @@ -84,7 +86,12 @@ export function ResponseCard({ revealing ? " markdown-stream" : "" }`} > - + ); @@ -96,12 +103,6 @@ export function ResponseCard({

{title}

- {streaming && ( - - - streaming - - )}
{markdown}
@@ -116,10 +117,12 @@ function ReactMarkdownInner({ body, sources, streamMs, + streamStartMs, }: { body: string; sources?: ProvenanceItem[]; streamMs?: number; + streamStartMs?: number; }) { // Memoized so streaming updates don't remount the answer and restart its // reveal. @@ -243,7 +246,9 @@ function ReactMarkdownInner({ diff --git a/narratives/ui/src/components/chip_citation.test.ts b/narratives/ui/src/components/chip_citation.test.ts index 6b81cf3ac..1d68e3a24 100644 --- a/narratives/ui/src/components/chip_citation.test.ts +++ b/narratives/ui/src/components/chip_citation.test.ts @@ -14,12 +14,15 @@ * permissions and limitations under the License. */ +import { isValidElement, type ReactNode } from "react"; import { describe, expect, it } from "vitest"; import { + CitationChip, citationAnchorId, displayCitationNumber, isCitable, sourceLabel, + splitWithCitations, } from "./chip_citation"; import type { ProvenanceItem } from "../hooks/use_sse_chat"; @@ -119,3 +122,78 @@ describe("citationAnchorId", () => { expect(citationAnchorId(2, 4)).toBe(citationAnchorId(2, 4)); }); }); + +/** Reads the citation number from a CitationChip element, or returns the node as-is. */ +function readChipNumberOrText(node: ReactNode): number | ReactNode { + if (isValidElement<{ n: number }>(node) && node.type === CitationChip) { + return node.props.n; + } + return node; +} + +describe("splitWithCitations", () => { + it("renders one chip per number of a composite marker", () => { + // Test: Composite citation markers. + // Situation: A figure derived from two tables carries `[1, 2]`, a later + // figure carries `[3]`, and two markers sit side by side. + // Expectation: `[1, 2]` becomes a chip for 1, a space, and a chip for 2, + // each single marker becomes one chip, and the text between the markers + // is kept in place. + const nodes = splitWithCitations( + "Rate rose [1, 2] and fell [3][1].", + SOURCES, + ); + + expect(nodes.map(readChipNumberOrText)).toEqual([ + "Rate rose ", + 1, + " ", + 2, + " and fell ", + 3, + 1, + ".", + ]); + }); + + it("accepts spacing around the comma of a composite marker", () => { + // Test: Marker spacing. + // Situation: Composite markers formatted without spaces (`[1,2]`) and with + // spaces around the comma (`[2 , 3]`). + // Expectation: Both split into one chip per number. + const tight = splitWithCitations("a [1,2] b", SOURCES); + const loose = splitWithCitations("a [2 , 3] b", SOURCES); + + expect(tight.map(readChipNumberOrText)).toEqual(["a ", 1, " ", 2, " b"]); + expect(loose.map(readChipNumberOrText)).toEqual(["a ", 2, " ", 3, " b"]); + }); + + it("renders one chip for a marker that repeats a number", () => { + // Test: Repeated numbers inside one marker. + // Situation: Prose carries the marker `[1, 1]`. + // Expectation: A single chip for 1, with no separator. + const nodes = splitWithCitations("a [1, 1] b", SOURCES); + + expect(nodes.map(readChipNumberOrText)).toEqual(["a ", 1, " b"]); + }); + + it("drops every number of a composite marker when citations are hidden", () => { + // Test: Hidden markers. + // Situation: A table cell whose column citation was hoisted onto the + // header holds a value and the composite marker `[1, 2]`. + // Expectation: The value stays and no chip or separator is rendered. + const nodes = splitWithCitations("value [1, 2]", SOURCES, true); + + expect(nodes).toEqual(["value "]); + }); + + it("leaves text without a marker untouched", () => { + // Test: The no-marker path. + // Situation: Plain text, and text with brackets that are not a marker. + // Expectation: The input string comes back as the only node. + expect(splitWithCitations("no markers", SOURCES)).toEqual(["no markers"]); + expect(splitWithCitations("see [a] and [1, b]", SOURCES)).toEqual([ + "see [a] and [1, b]", + ]); + }); +}); diff --git a/narratives/ui/src/components/chip_citation.tsx b/narratives/ui/src/components/chip_citation.tsx index 66d8ca0a9..82b765db3 100644 --- a/narratives/ui/src/components/chip_citation.tsx +++ b/narratives/ui/src/components/chip_citation.tsx @@ -18,8 +18,12 @@ * @fileoverview Renders an inline numbered citation chip. */ -import { createContext, useContext } from "react"; -import type { ProvenanceItem } from "../hooks/use_sse_chat"; +import { createContext, useContext, type ReactNode } from "react"; +import type { ProvenanceItem } from "@/src/hooks/use_sse_chat"; +import { + createCitationMarkerPattern, + parseCitationNumbers, +} from "@/src/utils/citation_marker"; import { renderNumbers } from "./value_numeric"; import { Tooltip } from "./tooltip"; @@ -193,17 +197,18 @@ function ChildWithCitations({ } /** - * Split a string into a mixed array of text fragments, shortened numbers and - * CitationChip elements, matching `[N]` where N is 1-99. Handles multiple - * adjacent citations like `[1] [2]` or `[1][2]`. + * Splits a string into a mixed array of text fragments, shortened numbers, and + * CitationChip elements, rendering one chip per number in each marker so a + * composite `[1, 2]` links to both of its sources. Handles multiple adjacent + * citations like `[1] [2]` or `[1][2]`. */ -function splitWithCitations( +export function splitWithCitations( s: string, sources?: ProvenanceItem[], hideCitations = false, -): React.ReactNode[] { - const pattern = /\[(\d{1,2})\]/g; - const out: React.ReactNode[] = []; +): ReactNode[] { + const pattern = createCitationMarkerPattern(); + const out: ReactNode[] = []; let last = 0; let match: RegExpExecArray | null; let i = 0; @@ -212,9 +217,12 @@ function splitWithCitations( out.push(...renderNumbers(s.slice(last, match.index), `n${last}`)); } if (!hideCitations) { - out.push( - , - ); + parseCitationNumbers(match[1]).forEach((n, position) => { + if (position > 0) { + out.push(" "); + } + out.push(); + }); } last = match.index + match[0].length; } diff --git a/narratives/ui/src/components/data_agent.test.tsx b/narratives/ui/src/components/data_agent.test.tsx index 1b8a186e2..5c3ae1a54 100644 --- a/narratives/ui/src/components/data_agent.test.tsx +++ b/narratives/ui/src/components/data_agent.test.tsx @@ -20,14 +20,21 @@ */ import { readFileSync } from 'node:fs'; -import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { + act, + cleanup, + fireEvent, + render, + screen, +} from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ChatSessionContext, type ChatSessionContextValue, } from '@/src/hooks/chat_session_context'; import type { ChatTurn } from '@/src/hooks/use_sse_chat'; -import { DataAgent, HANDOFF_MS } from './data_agent'; +import { DataAgent, HANDOFF_MS, TurnView } from './data_agent'; +import { ANSWER_SETTLE_MAX_MS } from './panel_answer'; class NoopResizeObserver { observe = vi.fn(); @@ -42,6 +49,7 @@ beforeEach(() => { afterEach(() => { cleanup(); vi.unstubAllGlobals(); + vi.useRealTimers(); }); /** Returns the surface over a settled chat state holding `turns`. */ @@ -83,6 +91,7 @@ describe('DataAgent refused and clarification turns', () => { toolCalls: [], thoughts: [], text: '', + blocks: [], provenance: [], }, ]); @@ -97,17 +106,21 @@ describe('DataAgent refused and clarification turns', () => { it('omits the completed reasoning placeholder on a clarification turn with no data calls', () => { // Test: Rendering a completed clarification turn in DataAgent. - // Situation: A turn completes with a clarification question in `text`, - // while `thoughts`, `toolCalls`, and `provenance` are all empty. + // Situation: A turn completes with a clarification question as its only + // text block, while `thoughts`, `toolCalls`, and `provenance` + // are all empty. // Expectation: The clarification text is rendered and the completed // ReasoningBlock placeholder is omitted. + const clarification = + 'Are you referring to the state of Georgia in the United States or the country of Georgia?'; renderTurns([ { userMessage: 'What is the population of Georgia?', status: 'done', toolCalls: [], thoughts: [], - text: 'Are you referring to the state of Georgia in the United States or the country of Georgia?', + text: clarification, + blocks: [{ index: 0, type: 'text', markdown: clarification }], provenance: [], }, ]); @@ -124,6 +137,49 @@ describe('DataAgent refused and clarification turns', () => { ), ).toBeNull(); }); + + it('shows the error and no answer panel for an error turn with only chart blocks', () => { + // Test: A failed turn that streamed charts but no prose. + // Situation: A turn ends in `error` holding a single chart block and no + // answer text. + // Expectation: The error message renders and no answer panel, chart, or + // Markdown body does. + const { container } = render( + , + ); + + expect( + screen.getByText('The agent failed to finish the answer.'), + ).toBeDefined(); + expect(container.querySelector('.max-w-4xl.grid')).toBeNull(); + expect(container.querySelector('.dc-chart-card')).toBeNull(); + expect(container.querySelector('.markdown-body')).toBeNull(); + }); }); describe('DataAgent New chat', () => { @@ -140,6 +196,7 @@ describe('DataAgent New chat', () => { toolCalls: [], thoughts: [], text: 'About 68 million.', + blocks: [{ index: 0, type: 'text', markdown: 'About 68 million.' }], provenance: [], }, ]); @@ -176,6 +233,7 @@ describe('DataAgent new-turn scroll', () => { toolCalls: [], thoughts: [], text: 'About 68 million.', + blocks: [{ index: 0, type: 'text', markdown: 'About 68 million.' }], provenance: [], }; const { rerender } = render(
{chatSurface([turn])}
); @@ -207,4 +265,71 @@ describe('DataAgent skeleton handoff', () => { expect(duration).not.toBeNull(); expect(Number(duration?.[1])).toBe(HANDOFF_MS); }); + + it('holds the answer invisible behind the skeleton until it settles', () => { + // Test: The skeleton handoff after synthesis. + // Situation: A streaming turn in the synthesis phase already holds a + // text block and a chart block, then finishes, and its chart never + // draws. + // Expectation: During synthesis only the skeleton renders. Once the turn + // is done the answer mounts invisible and aria-hidden behind a fully + // visible skeleton; when the settle cap passes, the answer shows and + // the skeleton fades. + vi.useFakeTimers(); + const synthesizing: ChatTurn = { + userMessage: 'What is the population of France?', + status: 'synthesis', + toolCalls: [], + thoughts: [], + text: 'France has about 68 million people.', + blocks: [ + { + index: 0, + type: 'text', + markdown: 'France has about 68 million people.', + }, + { + index: 1, + type: 'chart', + chart: { + vizType: 'line', + title: 'Population, France', + variableDcids: ['Count_Person'], + placeDcids: ['country/FRA'], + }, + }, + ], + provenance: [], + }; + const { container, rerender } = render( + , + ); + + expect(container.querySelector('.bg-skeleton-bar')).not.toBeNull(); + expect(container.querySelector('.markdown-body')).toBeNull(); + + rerender( + , + ); + const cell = container.querySelector('.max-w-4xl.grid'); + const [skeleton, answer] = Array.from(cell?.children ?? []); + expect(skeleton.querySelector('.bg-skeleton-bar')).not.toBeNull(); + expect(skeleton.classList.contains('opacity-0')).toBe(false); + expect(answer.querySelector('.markdown-body')).not.toBeNull(); + expect(answer.classList.contains('invisible')).toBe(true); + expect(answer.getAttribute('aria-hidden')).toBe('true'); + + act(() => { + vi.advanceTimersByTime(ANSWER_SETTLE_MAX_MS); + }); + + expect(answer.classList.contains('invisible')).toBe(false); + expect(answer.getAttribute('aria-hidden')).toBeNull(); + expect(skeleton.classList.contains('opacity-0')).toBe(true); + }); }); diff --git a/narratives/ui/src/components/data_agent.tsx b/narratives/ui/src/components/data_agent.tsx index 4182c4208..caf5cb20c 100644 --- a/narratives/ui/src/components/data_agent.tsx +++ b/narratives/ui/src/components/data_agent.tsx @@ -18,15 +18,19 @@ * @fileoverview Data Agent chat surface: prompt input, streaming turns, stop control, and follow-up handling. */ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { InitialView } from "./view_initial"; import { SkeletonCard } from "./card_skeleton"; import { ReasoningBlock } from "./block_reasoning"; import { AnswerPanel } from "./panel_answer"; import { DISCLAIMER_TEXT } from "./note_disclaimer"; import { PromptInput } from "./input_prompt"; -import { assignCitationNumbers, type ChatTurn } from "../hooks/use_sse_chat"; -import { useChatSession } from "../hooks/chat_session_context"; +import { + assignCitationNumbers, + type ChatTurn, + hasAnswer, +} from "@/src/hooks/use_sse_chat"; +import { useChatSession } from "@/src/hooks/chat_session_context"; /** How long the scroll to a new question glides. */ const PIN_GLIDE_MS = 400; @@ -226,7 +230,7 @@ interface TurnViewProps { } /** One conversation turn: the user's bubble plus the streamed agent response. */ -function TurnView({ +export function TurnView({ turn, index, citationNumbers, @@ -234,10 +238,13 @@ function TurnView({ isStreaming, onAsk, }: TurnViewProps) { - // The answer stays behind the skeleton until its text has arrived in full. - const textReady = - turn.textComplete || turn.status === "done" || turn.status === "error"; - const showAnswer = Boolean(turn.text) && !turn.stopped && textReady; + // The answer stays behind the skeleton until the turn ends (`done` or + // `error`), so the whole answer reveals at once in reading order. A turn + // with no answer text shows no answer, even when it holds chart blocks. + const showAnswer = + hasAnswer(turn) && + !turn.stopped && + (turn.status === "done" || turn.status === "error"); const showSkeleton = !showAnswer && !turn.stopped && @@ -247,13 +254,28 @@ function TurnView({ // Keep a shown skeleton under the answer until the crossfade ends. const [hadSkeleton, setHadSkeleton] = useState(false); if (showSkeleton && !hadSkeleton) setHadSkeleton(true); + // The answer mounts invisibly behind the skeleton so its charts load and + // take their height before anything is shown; the crossfade starts once + // the panel reports it has settled. + const [isAnswerSettled, setIsAnswerSettled] = useState(false); + const markAnswerSettled = useCallback(() => setIsAnswerSettled(true), []); + const isAnswerSettling = showAnswer && hadSkeleton && !isAnswerSettled; + const isHandingOff = showAnswer && hadSkeleton && isAnswerSettled; const [handedOff, setHandedOff] = useState(false); useEffect(() => { - if (!showAnswer || !hadSkeleton) return; + if (!isHandingOff) return; const timer = window.setTimeout(() => setHandedOff(true), HANDOFF_MS); return () => window.clearTimeout(timer); - }, [showAnswer, hadSkeleton]); + }, [isHandingOff]); const crossfading = showAnswer && hadSkeleton && !handedOff; + // `invisible` keeps the settling answer laid out, so its charts load, while + // the skeleton in the same grid cell stays the only thing shown. + let answerClassName: string | undefined; + if (isAnswerSettling) { + answerClassName = "invisible"; + } else if (hadSkeleton) { + answerClassName = "motion-safe:animate-stream-in"; + } const className = [ "shrink-0 flex flex-col gap-6", COLUMN, @@ -314,15 +336,17 @@ function TurnView({ Loading: `discovery` through `synthesis`; not during `classify`, which may still refuse. - Answer: AnswerPanel once the text is whole, crossfading over the - skeleton in a shared grid cell. Hidden when stopped. */} + Answer: AnswerPanel once the turn ends (`done` or `error`) with + answer text, mounted invisibly behind the skeleton in a shared grid + cell until its charts settle, then crossfading over it. Hidden when + stopped. */} {(showSkeleton || showAnswer) && (
{(showSkeleton || crossfading) && (
@@ -331,11 +355,8 @@ function TurnView({ )} {showAnswer && (
)} diff --git a/narratives/ui/src/components/panel_answer.test.tsx b/narratives/ui/src/components/panel_answer.test.tsx new file mode 100644 index 000000000..e69eb851f --- /dev/null +++ b/narratives/ui/src/components/panel_answer.test.tsx @@ -0,0 +1,367 @@ +/** + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +/** + * @fileoverview Tests for how the answer panel renders a turn's text and + * chart blocks, waits for its charts to settle, and reveals its text. + */ + +import { act, cleanup, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { ChartItem, ChatTurn } from '@/src/hooks/use_sse_chat'; +import { calculateStreamDurationMs } from '@/src/utils/rehype_stream_words'; +import { ANSWER_SETTLE_MAX_MS, AnswerPanel } from './panel_answer'; + +const LINE_CHART: ChartItem = { + vizType: 'line', + title: 'Population, France', + variableDcids: ['Count_Person'], + placeDcids: ['country/FRA'], +}; + +/** How often the chart settle poll reads the component, in milliseconds. */ +const CHART_POLL_MS = 300; + +/** Class that hides the body's non-text children while the text reveals. */ +const HIDE_NON_TEXT_CLASS = 'motion-safe:[&>:not(.markdown-body)]:opacity-0'; + +/** Builds a finished turn whose answer arrived as the given blocks. */ +const buildTurnWithBlocks = ( + blocks: ChatTurn['blocks'], + overrides: Partial = {}, +): ChatTurn => ({ + userMessage: 'What is the population of France?', + status: 'done', + toolCalls: [], + thoughts: [], + text: blocks + .flatMap((block) => (block.type === 'text' ? [block.markdown] : [])) + .join('\n\n'), + blocks, + provenance: [], + ...overrides, +}); + +/** A paragraph, a chart, and a second paragraph. */ +const INTERLEAVED_BLOCKS: ChatTurn['blocks'] = [ + { index: 0, type: 'text', markdown: 'France has about 68 million people.' }, + { index: 1, type: 'chart', chart: LINE_CHART }, + { index: 2, type: 'text', markdown: 'The population grew 0.3% last year.' }, +]; + +/** Reads the answer's text bodies and chart cards in document order. */ +const readAnswerParts = (container: HTMLElement): string[] => + Array.from(container.querySelectorAll('.markdown-body, .dc-chart-card')).map( + (node) => + node.classList.contains('dc-chart-card') + ? `chart:${node.querySelector('[header]')?.getAttribute('header')}` + : `text:${node.textContent?.trim()}`, + ); + +/** Gives the mounted line chart a shadow root holding a drawn chart. */ +const drawLineChart = (container: HTMLElement): void => { + const element = container.querySelector('datacommons-line'); + if (!element) throw new Error('No datacommons-line element is mounted.'); + element.attachShadow({ mode: 'open' }).innerHTML = + '
'; +}; + +/** Returns the panel body that holds the answer blocks. */ +const getBody = (container: HTMLElement): HTMLElement => { + const body = container.querySelector('.markdown-body')?.parentElement; + if (!body) throw new Error('No answer body is rendered.'); + return body; +}; + +afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +describe('AnswerPanel', () => { + it('renders text and chart blocks in the order they arrived', () => { + // Test: Interleaved block rendering. + // Situation: A finished turn whose blocks are a paragraph, a chart, and + // a second paragraph. + // Expectation: The answer body holds a Markdown body for each paragraph + // and a chart card for the chart, in that order, as direct children of + // the body, and the chart card mounts the chart's web component. + const { container } = render( + , + ); + + expect(readAnswerParts(container)).toEqual([ + 'text:France has about 68 million people.', + 'chart:Population, France', + 'text:The population grew 0.3% last year.', + ]); + const body = getBody(container); + for (const markdown of container.querySelectorAll('.markdown-body')) { + expect(markdown.parentElement).toBe(body); + } + expect(container.querySelector('datacommons-line')).not.toBeNull(); + }); + + it('holds the reveal while a chart is settling', () => { + // Test: The settling phase. + // Situation: A panel mounts mid-stream with a chart block that has not + // drawn yet. + // Expectation: `onSettled` has not fired and no text block carries the + // reveal animation class. + vi.useFakeTimers(); + const onSettled = vi.fn(); + const { container } = render( + , + ); + + expect(onSettled).not.toHaveBeenCalled(); + expect(container.querySelectorAll('.markdown-body')).toHaveLength(2); + expect(container.querySelector('.markdown-stream')).toBeNull(); + }); + + it('settles at once when the answer has no charts', () => { + // Test: A text-only answer. + // Situation: A panel mounts mid-stream with two text blocks and no + // chart blocks. + // Expectation: `onSettled` fires once and the text starts revealing. + const onSettled = vi.fn(); + const { container } = render( + , + ); + + expect(onSettled).toHaveBeenCalledTimes(1); + expect(container.querySelectorAll('.markdown-stream')).toHaveLength(2); + }); + + it('ends settling when its chart reports settled', () => { + // Test: A chart that draws before the cap. + // Situation: A panel mounts mid-stream with one chart block, and the + // chart draws before the next settle poll. + // Expectation: `onSettled` fires once after that poll, well before the + // cap, and the text starts revealing. + vi.useFakeTimers(); + const onSettled = vi.fn(); + const { container } = render( + , + ); + + drawLineChart(container); + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS); + }); + + expect(onSettled).toHaveBeenCalledTimes(1); + expect(container.querySelectorAll('.markdown-stream')).toHaveLength(2); + }); + + it('ends settling at the cap when a chart never settles', () => { + // Test: The settle cap. + // Situation: A panel mounts mid-stream with a chart that never draws. + // Expectation: `onSettled` has not fired just before + // ANSWER_SETTLE_MAX_MS and fires once when it passes. + vi.useFakeTimers(); + const onSettled = vi.fn(); + render( + , + ); + + act(() => { + vi.advanceTimersByTime(ANSWER_SETTLE_MAX_MS - 1); + }); + expect(onSettled).not.toHaveBeenCalled(); + + act(() => { + vi.advanceTimersByTime(1); + }); + expect(onSettled).toHaveBeenCalledTimes(1); + }); + + it('waits for its charts when the stream has already closed', () => { + // Test: A handoff from the skeleton after the stream closed. + // Situation: The panel mounts with `isStreaming` false but with an + // `onSettled` caller, and its chart has not drawn yet. + // Expectation: `onSettled` waits for the chart to draw, and the text + // then reveals word by word as in any other handoff. + vi.useFakeTimers(); + const onSettled = vi.fn(); + const { container } = render( + , + ); + expect(onSettled).not.toHaveBeenCalled(); + + drawLineChart(container); + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS); + }); + + expect(onSettled).toHaveBeenCalledTimes(1); + expect(container.querySelectorAll('.markdown-stream')).toHaveLength(2); + }); + + it('reveals at once when no caller waits on it', () => { + // Test: A mid-stream panel with no `onSettled` caller. + // Situation: The panel mounts with `isStreaming` true, no `onSettled`, + // and a chart block that has not drawn. + // Expectation: The text carries the reveal animation class immediately, + // without any timer advancing. + vi.useFakeTimers(); + const { container } = render( + , + ); + + expect(container.querySelector('.markdown-stream')).not.toBeNull(); + }); + + it('starts each text block where the previous one ended', () => { + // Test: One reveal across text blocks. + // Situation: A text-only panel mounts mid-stream with a three-word block + // and a two-word block, five words revealed over 100ms. + // Expectation: The first block's words start at 0ms, 20ms, and 40ms, and + // the second block's first word starts at 60ms. + const { container } = render( + , + ); + + const [first, second] = Array.from( + container.querySelectorAll('.markdown-body'), + ); + const readDelays = (body: Element): string[] => + Array.from(body.querySelectorAll('.stream-word')).map( + (word) => word.style.animationDelay, + ); + expect(readDelays(first)).toEqual(['0ms', '20ms', '40ms']); + expect(readDelays(second)[0]).toBe('60ms'); + }); + + it('shows everything at once when it neither reveals nor is awaited', () => { + // Test: A panel that mounts after the stream closed, with no caller + // waiting on it. + // Situation: `isStreaming` is false and no `onSettled` is given. + // Expectation: Without advancing any timer, no text block carries the + // reveal animation, the body does not hide its non-text children, and + // the Export PDF control renders. + vi.useFakeTimers(); + const { container } = render( + , + ); + + expect(container.querySelector('.markdown-stream')).toBeNull(); + expect(getBody(container).className).not.toContain(HIDE_NON_TEXT_CLASS); + expect( + screen.queryAllByRole('button', { name: 'Export PDF' }).length, + ).toBeGreaterThan(0); + }); + + it('shows the whole card at once under reduced motion', () => { + // Test: The reveal under `prefers-reduced-motion: reduce`. + // Situation: The user prefers reduced motion, and a text-only panel + // mounts mid-stream on a finished turn. + // Expectation: Without advancing any timer, no text block carries the + // reveal animation, the body does not hide its non-text children, and + // the Export PDF control renders. + vi.useFakeTimers(); + vi.stubGlobal( + 'matchMedia', + (query: string): Pick => ({ + matches: query === '(prefers-reduced-motion: reduce)', + media: query, + }), + ); + const { container } = render( + , + ); + + expect(container.querySelector('.markdown-stream')).toBeNull(); + expect(getBody(container).className).not.toContain(HIDE_NON_TEXT_CLASS); + expect( + screen.queryAllByRole('button', { name: 'Export PDF' }).length, + ).toBeGreaterThan(0); + }); + + it('hides non-text children and the export controls until the reveal ends', () => { + // Test: Hiding during the reveal. + // Situation: A text-only panel mounts mid-stream on a finished turn, so + // it reveals at once. + // Expectation: While the text reveals, the body hides its non-text + // children and no Export PDF control renders; once the reveal duration + // passes, the hiding class is gone and the export controls render. + vi.useFakeTimers(); + const turn = buildTurnWithBlocks([ + { index: 0, type: 'text', markdown: 'France has about 68 million.' }, + ]); + const { container } = render(); + + expect(getBody(container).className).toContain(HIDE_NON_TEXT_CLASS); + expect( + screen.queryAllByRole('button', { name: 'Export PDF' }), + ).toHaveLength(0); + + act(() => { + vi.advanceTimersByTime(calculateStreamDurationMs(turn.text)); + }); + + expect(getBody(container).className).not.toContain(HIDE_NON_TEXT_CLASS); + expect( + screen.queryAllByRole('button', { name: 'Export PDF' }).length, + ).toBeGreaterThan(0); + }); +}); diff --git a/narratives/ui/src/components/panel_answer.tsx b/narratives/ui/src/components/panel_answer.tsx index 8cd8f0887..448b0f592 100644 --- a/narratives/ui/src/components/panel_answer.tsx +++ b/narratives/ui/src/components/panel_answer.tsx @@ -18,8 +18,12 @@ * @fileoverview Renders a completed answer as a side-panel card: markdown body, sources, charts, disclaimer, export, and follow-ups. */ -import { useEffect, useMemo, useRef, useState } from "react"; -import type { ChatTurn } from "../hooks/use_sse_chat"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { + type AnswerBlock, + type ChatTurn, + getTurnCharts, +} from "@/src/hooks/use_sse_chat"; import { ChartTile } from "./tile_chart"; import { DisclaimerNote } from "./note_disclaimer"; import { @@ -33,8 +37,11 @@ import { ResponseCard } from "./card_response"; import { Tooltip } from "./tooltip"; import { CitationNumberingProvider } from "./chip_citation"; import { CitationsList } from "./list_citations"; -import { downloadPdf } from "../utils/download_pdf"; -import { calculateStreamDurationMs } from "../utils/rehype_stream_words"; +import { downloadPdf } from "@/src/utils/download_pdf"; +import { + calculateStreamDurationMs, + countWords, +} from "@/src/utils/rehype_stream_words"; import { Toast } from "./toast"; import { CardHeader } from "./card_header"; @@ -59,8 +66,47 @@ interface AnswerPanelProps { citationNumbers?: number[]; /** Which turn this is, so its anchors cannot collide with another's. */ turnIndex?: number; + /** + * Called once every chart has settled or {@link ANSWER_SETTLE_MAX_MS} has + * passed. Passing it makes the panel wait for its charts before it reports + * settled, so the caller can keep it hidden until its layout is final. + */ + onSettled?: () => void; +} + +/** + * How far the panel's reveal has progressed. `settling`: laid out but waiting for + * its charts to finish loading. Charts that settle within + * {@link ANSWER_SETTLE_MAX_MS} move nothing once the panel is shown; a chart + * that draws or reports no data after the cap can still change its height. + * `revealing`: the text is revealing word by word and everything else is + * hidden. `revealed`: everything is shown. + */ +type RevealPhase = "settling" | "revealing" | "revealed"; + +/** + * Returns the phase a panel mounts in. It settles first only when a caller + * waits on `onSettled`; otherwise it reveals at once when it should reveal + * and is shown in full when it should not. + */ +function getInitialPhase( + shouldReveal: boolean, + isAwaited: boolean, +): RevealPhase { + if (isAwaited) return "settling"; + if (shouldReveal) return "revealing"; + return "revealed"; } +/** Media query for the user's reduced-motion preference. */ +const REDUCED_MOTION_QUERY = "(prefers-reduced-motion: reduce)"; + +/** + * Longest the panel waits for its charts to settle before revealing anyway, + * in milliseconds, so a slow chart cannot hold back the answer. + */ +export const ANSWER_SETTLE_MAX_MS = 2000; + const COLOR_BORDER = "var(--color-border)"; /** * Per Figma 3427-16728 the content body fills the narrative surface — light gray in the @@ -85,22 +131,70 @@ export function AnswerPanel({ onAsk, citationNumbers, turnIndex, + onSettled, }: AnswerPanelProps) { const panelRef = useRef(null); - const hasCharts = !!turn.chartConfig; - // A panel that mounts mid-stream is taking over from the skeleton, so its - // text reveals in reading order; one restored with its session is already - // there. Everything below the text waits for the reveal to finish. - const [enters] = useState(isStreaming); - const streamMs = enters ? calculateStreamDurationMs(turn.text) : undefined; - const [revealing, setRevealing] = useState(enters); + const hasCharts = getTurnCharts(turn).length > 0; + // A panel that mounts mid-stream, or whose caller waits on `onSettled`, is + // taking over from the skeleton, so it reveals its text in reading order. + // The caller check covers a stream that closed in the same render as the + // terminal event. A panel with a waiting caller settles its charts first, + // so charts that settle within the cap load while hidden. Under reduced + // motion there is no word reveal, so the whole card, export controls + // included, shows as soon as it settles. + const [shouldReveal] = useState( + () => + (isStreaming || onSettled !== undefined) && + !window.matchMedia(REDUCED_MOTION_QUERY).matches, + ); + const [phase, setPhase] = useState(() => + getInitialPhase(shouldReveal, onSettled !== undefined), + ); + const streamMs = shouldReveal + ? calculateStreamDurationMs(turn.text) + : undefined; + const [settledChartIndices, setSettledChartIndices] = useState< + ReadonlySet + >(() => new Set()); + const markChartSettled = useCallback((index: number) => { + setSettledChartIndices((prev) => { + if (prev.has(index)) return prev; + const next = new Set(prev); + next.add(index); + return next; + }); + }, []); + // True at once for a turn with no chart blocks. + const areChartsSettled = turn.blocks.every( + (block) => block.type !== "chart" || settledChartIndices.has(block.index), + ); useEffect(() => { - if (streamMs === undefined) return; - const timer = window.setTimeout(() => setRevealing(false), streamMs); + if (phase !== "settling") return; + const nextPhase: RevealPhase = shouldReveal ? "revealing" : "revealed"; + if (areChartsSettled) { + setPhase(nextPhase); + return; + } + const timer = window.setTimeout( + () => setPhase(nextPhase), + ANSWER_SETTLE_MAX_MS, + ); return () => window.clearTimeout(timer); - }, [streamMs]); - const settled = - !isStreaming || turn.status === "done" || turn.status === "error"; + }, [phase, areChartsSettled, shouldReveal]); + useEffect(() => { + if (phase !== "revealing" || streamMs === undefined) return; + const timer = window.setTimeout(() => setPhase("revealed"), streamMs); + return () => window.clearTimeout(timer); + }, [phase, streamMs]); + // Held in a ref so an inline arrow from the caller does not re-fire the + // effect; `isSettling` changes at most once, so the call happens once. + const onSettledRef = useRef(onSettled); + onSettledRef.current = onSettled; + const isSettling = phase === "settling"; + useEffect(() => { + if (!isSettling) onSettledRef.current?.(); + }, [isSettling]); + const isRevealed = phase === "revealed"; /** * `turn.provenance` is the sources that actually supplied the answer's * numbers. The backend decides membership from which facet each observation @@ -158,7 +252,7 @@ export function AnswerPanel({ produces a PDF missing the chart. This matches the filled "Export PDF" button below, which also waits for status === "done". */} - {turn.status === "done" && !revealing && ( + {turn.status === "done" && isRevealed && ( <> {/* Opens the provenance slide-over. Only under ?debug=provenance, so end users never see it. */} @@ -168,37 +262,26 @@ export function AnswerPanel({ )} - {/* Content body — dimmed while the turn is in flight and brought to full - strength once its terminal event lands. Everything after the text - stays hidden until the text has finished revealing. */} + {/* Content body: everything after and between the text blocks stays + hidden until the text has finished revealing. */}
:not(.markdown-body)]:transition-opacity motion-safe:[&>:not(.markdown-body)]:duration-300${ - revealing ? " motion-safe:[&>:not(.markdown-body)]:opacity-0" : "" + className={`flex flex-col p-4 sm:p-6 motion-safe:[&>:not(.markdown-body)]:transition-opacity motion-safe:[&>:not(.markdown-body)]:duration-300${ + isRevealed ? "" : " motion-safe:[&>:not(.markdown-body)]:opacity-0" }`} style={{ gap: 16, background: COLOR_BODY_BG, }} > - {/* 1. Markdown body — bare (no nested card chrome) */} - - {/* 2. Charts */} - {hasCharts && ( - - )} - - {/* 3. Truncation notice — the agent ran out of research steps, so + {/* 2. Truncation notice: the agent ran out of research steps, so this answer rests on less data than it meant to gather. Not styled as an error: the answer is usable, just less complete. */} {turn.truncated && ( @@ -211,7 +294,7 @@ export function AnswerPanel({

)} - {/* 4. Citations — at the end of the document, above the disclaimer, + {/* 3. Citations: at the end of the document, above the disclaimer, where a reader who has finished the answer looks for them. */} {hasCitations && ( )} - {/* 5. Disclaimer */} + {/* 4. Disclaimer */} {(hasCharts || hasCitations) && } - {/* 6. Export PDF (filled, in-content) */} - {turn.status === "done" && !revealing && ( + {/* 5. Export PDF (filled, in-content) */} + {turn.status === "done" && isRevealed && ( )} - {/* 7. Follow-up questions — agent-generated, self-contained. Hidden + {/* 6. Follow-up questions: agent-generated, self-contained. Hidden when the agent returned none (no static fallback). Marked non-print: they are buttons that ask a NEW question, so they @@ -250,6 +333,94 @@ export function AnswerPanel({ ); } +interface AnswerBlocksProps { + turn: ChatTurn; + phase: RevealPhase; + /** Duration of the whole text reveal; undefined when the panel does not reveal. */ + streamMs?: number; + onChartSettled: (index: number) => void; +} + +/** One text block's share of the reveal, in milliseconds. */ +interface TextRevealWindow { + startMs: number; + durationMs: number; +} + +/** + * Renders a turn's blocks in emission order, as direct children of the + * panel body: each text block through the Markdown renderer and each chart + * block through a one-item chart tile that reports when it has settled. The + * text blocks share one reveal in reading order, and the word animations + * start only once `phase` is `revealing`. + */ +function AnswerBlocks({ + turn, + phase, + streamMs, + onChartSettled, +}: AnswerBlocksProps) { + const blocks = turn.blocks; + const schedule = + streamMs === undefined ? undefined : scheduleTextReveal(blocks, streamMs); + return ( + <> + {blocks.map((block) => { + if (block.type === "chart") { + return ( + onChartSettled(block.index)} + /> + ); + } + const revealWindow = schedule?.get(block.index); + return ( + + ); + })} + + ); +} + +/** + * Splits one reveal of `totalMs` across the text blocks in reading order, in + * proportion to their word counts, so each block starts where the previous + * one ends. Returns each text block's window keyed by its block index. + */ +function scheduleTextReveal( + blocks: AnswerBlock[], + totalMs: number, +): Map { + const wordCounts = blocks.map((block) => + block.type === "text" ? countWords(block.markdown) : 0, + ); + const totalWords = wordCounts.reduce((sum, count) => sum + count, 0); + const msPerWord = totalMs / Math.max(totalWords, 1); + const schedule = new Map(); + let wordsBefore = 0; + blocks.forEach((block, position) => { + if (block.type !== "text") return; + const words = wordCounts[position]; + schedule.set(block.index, { + startMs: wordsBefore * msPerWord, + durationMs: words * msPerWord, + }); + wordsBefore += words; + }); + return schedule; +} + /** * Tonal "Export PDF" pill in the card's toolbar header, with the `output` * leading icon. Shares the tonal container/content tokens and the icon with diff --git a/narratives/ui/src/components/panel_debug_provenance.test.tsx b/narratives/ui/src/components/panel_debug_provenance.test.tsx new file mode 100644 index 000000000..16fbf9cd3 --- /dev/null +++ b/narratives/ui/src/components/panel_debug_provenance.test.tsx @@ -0,0 +1,113 @@ +/** + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +/** + * @fileoverview Tests for the provenance inspector's chart listing, which + * reads a turn's chart blocks. + */ + +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it } from 'vitest'; +import type { + ChartItem, + ChatTurn, + ToolCallEvent, +} from '@/src/hooks/use_sse_chat'; +import { ProvenanceInspector } from './panel_debug_provenance'; + +const FETCH_CALL: ToolCallEvent = { + name: 'fetch_observations', + arguments: { variable_dcid: 'Count_Person', place_dcids: ['country/FRA'] }, + status: 'success', +}; + +const LINE_CHART: ChartItem = { + vizType: 'line', + title: 'Population, France', + variableDcids: ['Count_Person'], + placeDcids: ['country/FRA'], +}; + +/** Builds a finished turn with one fetch call and the given overrides. */ +const buildFinishedTurn = (overrides: Partial = {}): ChatTurn => ({ + userMessage: 'What is the population of France?', + status: 'done', + toolCalls: [FETCH_CALL], + thoughts: [], + text: 'France has about 68 million people.', + blocks: [ + { index: 0, type: 'text', markdown: 'France has about 68 million people.' }, + ], + provenance: [], + ...overrides, +}); + +/** Renders the inspector for the turn and opens its slide-over. */ +const openInspector = (turn: ChatTurn) => { + render(); + fireEvent.click( + screen.getByLabelText('Inspect data sources for this answer'), + ); +}; + +afterEach(() => { + cleanup(); +}); + +describe('ProvenanceInspector', () => { + it('lists the charts from the turn chart blocks', () => { + // Test: The chart listing of a turn streamed as blocks. + // Situation: A finished turn holding a text block and a chart block. + // Expectation: The "Charts requested" section lists the chart block's + // title. + openInspector( + buildFinishedTurn({ + blocks: [ + { + index: 0, + type: 'text', + markdown: 'France has about 68 million people.', + }, + { index: 1, type: 'chart', chart: LINE_CHART }, + ], + }), + ); + + expect(screen.getByText('Charts requested')).toBeTruthy(); + expect(screen.getByText('Population, France')).toBeTruthy(); + }); + + it('omits the chart section for a turn without charts', () => { + // Test: The chart listing of a turn that rendered no chart. + // Situation: A finished turn whose only block is text. + // Expectation: No "Charts requested" section renders, while the tool + // calls section still does. + openInspector( + buildFinishedTurn({ + blocks: [ + { + index: 0, + type: 'text', + markdown: 'France has about 68 million people.', + }, + ], + }), + ); + + expect(screen.queryByText('Charts requested')).toBeNull(); + expect(screen.getByText('Tool calls (1)')).toBeTruthy(); + }); +}); diff --git a/narratives/ui/src/components/panel_debug_provenance.tsx b/narratives/ui/src/components/panel_debug_provenance.tsx index 88b8ec097..cbdd86fe0 100644 --- a/narratives/ui/src/components/panel_debug_provenance.tsx +++ b/narratives/ui/src/components/panel_debug_provenance.tsx @@ -20,8 +20,8 @@ */ import { useEffect, useRef, useState } from "react"; -import type { ChatTurn } from "../hooks/use_sse_chat"; -import { inspectTurn } from "../utils/inspect_turn"; +import { type ChatTurn, getTurnCharts } from "@/src/hooks/use_sse_chat"; +import { inspectTurn } from "@/src/utils/inspect_turn"; import { InspectIcon } from "./icons"; /** @@ -60,7 +60,7 @@ function Detail({ label, value }: { label: string; value?: string | number }) { /** The inspector's contents, without any chrome. */ function InspectorBody({ turn }: { turn: ChatTurn }) { const [openResult, setOpenResult] = useState(null); - const inspection = inspectTurn(turn.toolCalls, turn.chartConfig, turn.provenance); + const inspection = inspectTurn(turn.toolCalls, getTurnCharts(turn), turn.provenance); const usedFacets = inspection.facets.filter((facet) => facet.used); return ( diff --git a/narratives/ui/src/components/tile_chart.test.tsx b/narratives/ui/src/components/tile_chart.test.tsx new file mode 100644 index 000000000..c91640ac9 --- /dev/null +++ b/narratives/ui/src/components/tile_chart.test.tsx @@ -0,0 +1,436 @@ +/** + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +/** + * @fileoverview Tests for the chart tile: the attributes each chart item + * hands its Data Commons web component, and the tile's rendering of a + * one-item configuration. + */ + +import { act, cleanup, render } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { ChartItem } from '@/src/hooks/use_sse_chat'; +import type { DataCommonsComponentTagName } from '@/src/utils/datacommons_component'; +import { + ChartTile, + getDcComponentAttributes, + getDcComponentTag, +} from './tile_chart'; + +const GDP = 'Amount_EconomicActivity_GrossDomesticProduct_Nominal'; + +/** How often the chart settle poll reads the component, in milliseconds. */ +const CHART_POLL_MS = 300; + +/** + * How long the settle poll watches a pending component before giving up, in + * milliseconds; mirrors `SETTLE_BUDGET_MS` in `datacommons_component.tsx`. + */ +const SETTLE_BUDGET_MS = 20_000; + +/** Gives the mounted `tag` element a shadow root holding a drawn chart. */ +const drawChart = ( + container: HTMLElement, + tag: DataCommonsComponentTagName, +): void => { + const element = container.querySelector(tag); + if (!element) throw new Error(`No ${tag} element is mounted.`); + element.attachShadow({ mode: 'open' }).innerHTML = + '
'; +}; + +/** Gives the mounted `tag` element a shadow root holding the no-data notice. */ +const showNoData = ( + container: HTMLElement, + tag: DataCommonsComponentTagName, +): void => { + const element = container.querySelector(tag); + if (!element) throw new Error(`No ${tag} element is mounted.`); + element.attachShadow({ mode: 'open' }).innerHTML = + '
No data available.
'; +}; + +/** A line chart over one place, which the tile renders as one card. */ +const LINE_CHART: ChartItem = { + vizType: 'line', + title: 'GDP, France', + variableDcids: [GDP], + placeDcids: ['country/FRA'], +}; + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +describe('getDcComponentAttributes', () => { + it('sets the per-capita variable list under the name each component reads', () => { + // Test: Per-capita wiring. + // Situation: A per-capita bar chart, a per-capita scatter chart, and a + // per-capita slider, each over one or two variables. + // Expectation: The bar chart carries `perCapita`, the scatter chart + // carries `usePerCapita` instead, and the slider carries neither. + const bar = getDcComponentAttributes({ + vizType: 'bar', + title: 'GDP per person', + variableDcids: [GDP], + placeDcids: ['country/FRA', 'country/DEU'], + perCapita: true, + }); + const scatter = getDcComponentAttributes({ + vizType: 'scatter', + title: 'GDP against population', + variableDcids: [GDP, 'Count_Person'], + parentPlace: 'country/USA', + childPlaceType: 'State', + perCapita: true, + }); + const slider = getDcComponentAttributes({ + vizType: 'slider', + title: 'Slider', + variableDcids: [GDP], + placeDcids: ['country/FRA'], + perCapita: true, + }); + + expect(bar.perCapita).toBe(GDP); + expect(bar.usePerCapita).toBeUndefined(); + expect(scatter.usePerCapita).toBe(`${GDP} Count_Person`); + expect(scatter.perCapita).toBeUndefined(); + expect(slider.perCapita).toBeUndefined(); + expect(slider.usePerCapita).toBeUndefined(); + }); + + it('pins each variable to its served facet under the attribute the component reads', () => { + // Test: Facet-pinning wiring. + // Situation: A highlight tile with one facet, a two-variable bar chart + // where both variables carry facets, and a two-variable line chart + // where only one variable carries a facet. + // Expectation: The highlight tile receives `facetId`, the fully mapped + // bar chart receives `facetIds` in variable order, and the partially + // mapped line chart receives `facetMapping` as JSON. + const highlight = getDcComponentAttributes({ + vizType: 'highlight', + title: 'GDP', + variableDcids: [GDP], + placeDcids: ['country/FRA'], + facetIds: { [GDP]: 'f1' }, + }); + const bar = getDcComponentAttributes({ + vizType: 'bar', + title: 'Composition', + variableDcids: [GDP, 'Count_Person'], + placeDcids: ['country/FRA'], + facetIds: { Count_Person: 'f2', [GDP]: 'f1' }, + }); + const line = getDcComponentAttributes({ + vizType: 'line', + title: 'Partial facet map', + variableDcids: [GDP, 'Count_Person'], + placeDcids: ['country/FRA'], + facetIds: { [GDP]: 'f1' }, + }); + + expect(highlight.facetId).toBe('f1'); + expect(highlight.facetIds).toBeUndefined(); + expect(highlight.facetMapping).toBeUndefined(); + expect(bar.facetIds).toBe('f1 f2'); + expect(bar.facetId).toBeUndefined(); + expect(bar.facetMapping).toBeUndefined(); + expect(line.facetMapping).toBe(JSON.stringify({ [GDP]: 'f1' })); + expect(line.facetIds).toBeUndefined(); + }); + + it('passes the ranking and sort options through', () => { + // Test: Optional ranking and bar-sort display attributes. + // Situation: A ranking chart showing both ends of five places, a ranking + // chart showing only the lowest places, and a sorted bar chart. + // Expectation: Each option maps to the attribute of the same name, with + // true booleans set as bare attributes, false booleans omitted, and + // numbers kept as numbers. + const ranking = getDcComponentAttributes({ + vizType: 'ranking', + title: 'GDP by state', + variableDcids: [GDP], + parentPlace: 'country/USA', + childPlaceType: 'State', + rankingCount: 5, + showHighestLowest: true, + showLowest: false, + }); + const bottomRanking = getDcComponentAttributes({ + vizType: 'ranking', + title: 'Lowest GDP by state', + variableDcids: [GDP], + parentPlace: 'country/USA', + childPlaceType: 'State', + rankingCount: 3, + showLowest: true, + }); + const bar = getDcComponentAttributes({ + vizType: 'bar', + title: 'GDP', + variableDcids: [GDP], + placeDcids: ['country/FRA'], + sort: 'descending', + }); + + expect(ranking).toMatchObject({ + parentPlace: 'country/USA', + childPlaceType: 'State', + variable: GDP, + rankingCount: 5, + showHighestLowest: true, + }); + expect(ranking.showLowest).toBeUndefined(); + expect(bottomRanking.showLowest).toBe(true); + expect(bottomRanking.showHighestLowest).toBeUndefined(); + expect(bar.sort).toBe('descending'); + }); + + it('keeps the place, date, and unit attributes of a plain chart', () => { + // Test: The attributes every chart carries. + // Situation: A line chart over two places with a date and a unit, and + // no optional display settings. + // Expectation: The header, variables, places, singular place and + // variable, date, and unit are set, and no per-capita, facet, ranking, + // or sort attribute appears. + const attrs = getDcComponentAttributes({ + vizType: 'line', + title: 'GDP, France and Germany', + variableDcids: [GDP], + placeDcids: ['country/FRA', 'country/DEU'], + date: '2023', + unit: 'USD', + }); + + expect(attrs).toEqual({ + header: 'GDP, France and Germany', + variables: GDP, + places: 'country/FRA country/DEU', + place: 'country/FRA', + variable: GDP, + date: '2023', + unit: 'USD', + }); + }); +}); + +describe('getDcComponentTag', () => { + it('maps every chart kind to its component and unknown kinds to line', () => { + // Test: The tag mapping. + // Situation: Each chart kind the agent emits, plus `slider`, which has + // no mapping of its own. + // Expectation: Each kind maps to its `datacommons-*` element and the + // unmapped kind falls back to the line component. + const kinds: ChartItem['vizType'][] = [ + 'line', + 'bar', + 'map', + 'ranking', + 'highlight', + 'scatter', + ]; + + expect(kinds.map(getDcComponentTag)).toEqual([ + 'datacommons-line', + 'datacommons-bar', + 'datacommons-map', + 'datacommons-ranking', + 'datacommons-highlight', + 'datacommons-scatter', + ]); + expect(getDcComponentTag('slider')).toBe('datacommons-line'); + }); +}); + +describe('ChartTile', () => { + it('mounts one web component for a one-item configuration', () => { + // Test: Rendering a chart block's one-item configuration. + // Situation: A configuration holding one per-capita ranking chart over a + // cohort with a pinned facet. + // Expectation: One `datacommons-ranking` element is mounted inside a + // chart card, carrying the cohort, per-capita, and facet attributes. + const { container } = render( + , + ); + + const cards = container.querySelectorAll('.dc-chart-card'); + expect(cards).toHaveLength(1); + const element = cards[0].querySelector('datacommons-ranking'); + expect(element).not.toBeNull(); + expect(element?.getAttribute('parentPlace')).toBe('country/USA'); + expect(element?.getAttribute('childPlaceType')).toBe('State'); + expect(element?.getAttribute('perCapita')).toBe(GDP); + expect(element?.getAttribute('facetIds')).toBe('f1'); + expect(element?.getAttribute('header')).toBe('GDP per person by state'); + }); + + it('renders nothing for a chart without a place anchor', () => { + // Test: The under-specified chart guard. + // Situation: A configuration whose only chart names a variable but no + // places and no cohort. + // Expectation: The tile renders nothing rather than mounting a component + // that cannot fetch. + const { container } = render( + , + ); + + expect(container.querySelector('.dc-charts-container')).toBeNull(); + }); + + it('reports settled once, after every card has settled', () => { + // Test: The tile's settle report. + // Situation: A tile with a line chart and a bar chart; the line chart + // draws first and the bar chart draws one poll later. + // Expectation: `onSettled` has not fired while the bar chart is pending, + // fires once after it draws, and does not fire again. + vi.useFakeTimers(); + const onSettled = vi.fn(); + const { container } = render( + , + ); + + drawChart(container, 'datacommons-line'); + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS); + }); + expect(onSettled).not.toHaveBeenCalled(); + + drawChart(container, 'datacommons-bar'); + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS * 3); + }); + expect(onSettled).toHaveBeenCalledTimes(1); + }); + + it('reports settled at once when it renders nothing', () => { + // Test: The settle report of an empty tile. + // Situation: A configuration whose only chart has no place anchor, so + // the tile renders nothing. + // Expectation: `onSettled` fires once right after mount. + const onSettled = vi.fn(); + render( + , + ); + + expect(onSettled).toHaveBeenCalledTimes(1); + }); + + it('hides a chart that reports no data and reports settled once', () => { + // Test: The settle report of a chart with no data. + // Situation: A tile with one line chart whose shadow root holds the + // component's "No data available." notice. + // Expectation: After the next poll `onSettled` fires once, the card is + // hidden, and later polls do not fire it again. + vi.useFakeTimers(); + const onSettled = vi.fn(); + const { container } = render( + , + ); + + showNoData(container, 'datacommons-line'); + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS); + }); + expect(onSettled).toHaveBeenCalledTimes(1); + expect(container.querySelector('.dc-chart-card')).toBeNull(); + + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS * 3); + }); + expect(onSettled).toHaveBeenCalledTimes(1); + }); + + it('reports settled once when a chart stays pending past the settle budget', () => { + // Test: The settle report of a chart that never settles. + // Situation: A tile with one line chart whose component never attaches + // a shadow root, so it reads as pending on every poll. + // Expectation: `onSettled` has not fired before the settle budget runs + // out, fires once on the first poll after it, and does not fire again. + vi.useFakeTimers(); + const onSettled = vi.fn(); + render( + , + ); + + act(() => { + vi.advanceTimersByTime(SETTLE_BUDGET_MS - 1); + }); + expect(onSettled).not.toHaveBeenCalled(); + + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS); + }); + expect(onSettled).toHaveBeenCalledTimes(1); + + act(() => { + vi.advanceTimersByTime(CHART_POLL_MS * 3); + }); + expect(onSettled).toHaveBeenCalledTimes(1); + }); +}); diff --git a/narratives/ui/src/components/tile_chart.tsx b/narratives/ui/src/components/tile_chart.tsx index 02935c550..fdef29f99 100644 --- a/narratives/ui/src/components/tile_chart.tsx +++ b/narratives/ui/src/components/tile_chart.tsx @@ -18,15 +18,16 @@ * @fileoverview Renders a Data Commons web-component chart (line/bar/map/ranking/…) from an agent chart config. */ -import { useCallback, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; -import type { ChartConfig, ChartItem, ProvenanceItem } from "../hooks/use_sse_chat"; +import type { ChartConfig, ChartItem } from "@/src/hooks/use_sse_chat"; import { API_ROOT, DataCommonsComponent, + type DataCommonsComponentAttributes, type DataCommonsComponentTagName, useDcChartEmptyNotice, -} from "../utils/datacommons_component"; +} from "@/src/utils/datacommons_component"; /** * Chart renderer for chat answers. @@ -45,13 +46,18 @@ import { */ interface ChartTileProps { config: ChartConfig; - provenance?: ProvenanceItem[]; + /** + * Called once every rendered chart has settled (drawn, reported no data, or + * reached the watch budget of `useDcChartEmptyNotice`), and right after + * mount when the tile renders nothing. + */ + onSettled?: () => void; } const MAX_CHARTS = 3; /** Renders the agent's chart directive as a responsive grid of DC charts. */ -export function ChartTile({ config }: ChartTileProps) { +export function ChartTile({ config, onSettled }: ChartTileProps) { // Charts that reported no data of their own after mounting. The agent // already withholds chart configs when its tool calls returned no // observations, but each DC component runs its own fetch against the place @@ -72,27 +78,29 @@ export function ChartTile({ config }: ChartTileProps) { return next; }); }, []); + const [settledIndices, setSettledIndices] = useState>( + () => new Set(), + ); + const markSettled = useCallback((index: number) => { + setSettledIndices((prev) => { + if (prev.has(index)) return prev; + const next = new Set(prev); + next.add(index); + return next; + }); + }, []); - if (!config || !config.shouldRender || config.hideCharts) return null; - - // Legacy single-chart payloads are folded into `charts` upstream by - // mapRawChartConfig, so an empty list here means nothing to render. - const charts: ChartItem[] | undefined = config.charts; - if (!charts || !Array.isArray(charts) || charts.length === 0) { - return null; - } + const toRender = selectRenderableCharts(config); + // True at once for a tile with nothing to render. + const isSettled = toRender.every((_, index) => settledIndices.has(index)); + // Held in a ref so an inline arrow from the caller does not re-fire the + // effect on every parent re-render. + const onSettledRef = useRef(onSettled); + onSettledRef.current = onSettled; + useEffect(() => { + if (isSettled) onSettledRef.current?.(); + }, [isSettled]); - const toRender = charts - .slice(0, MAX_CHARTS) - .filter((chart) => chart.variableDcids && chart.variableDcids.length > 0) - // Each DC component needs SOME place anchor — either explicit places - // or a parent + child type. Drop entries the agent under-specified - // rather than letting the component crash. - .filter( - (chart) => - (chart.placeDcids && chart.placeDcids.length > 0) || - (chart.parentPlace && chart.childPlaceType), - ); if (toRender.length === 0) return null; const visibleCount = toRender.filter( @@ -119,12 +127,37 @@ export function ChartTile({ config }: ChartTileProps) { chart={chart} hidden={emptyIndices.has(index)} onEmpty={() => markEmpty(index)} + onSettled={() => markSettled(index)} /> ))}
); } +/** Returns the charts of `config` the tile can render; empty when none. */ +function selectRenderableCharts(config: ChartConfig): ChartItem[] { + if (!config || !config.shouldRender || config.hideCharts) return []; + + // The directive lists its charts in `charts`; an empty list means there is + // nothing to render. + const charts: ChartItem[] | undefined = config.charts; + if (!charts || !Array.isArray(charts) || charts.length === 0) { + return []; + } + + return charts + .slice(0, MAX_CHARTS) + .filter((chart) => chart.variableDcids && chart.variableDcids.length > 0) + // Each DC component needs SOME place anchor: either explicit places + // or a parent + child type. Drop entries the agent under-specified + // rather than letting the component crash. + .filter( + (chart) => + (chart.placeDcids && chart.placeDcids.length > 0) || + (chart.parentPlace && chart.childPlaceType), + ); +} + /** * Renders one chart as a Data Commons web component inside the card wrapper. * @@ -137,15 +170,17 @@ function ChartCard({ chart, hidden, onEmpty, + onSettled, }: { chart: ChartItem; hidden: boolean; onEmpty: () => void; + onSettled: () => void; }) { const cardRef = useRef(null); const tag = getDcComponentTag(chart.vizType); const attrs = getDcComponentAttributes(chart); - useDcChartEmptyNotice(cardRef, tag, onEmpty); + useDcChartEmptyNotice(cardRef, tag, onEmpty, onSettled); // The DC web component renders its own bordered card (Source caption, // chart, footer with "About this data" + "API code"). Wrapping it in // another bordered div produced a double-border look. We keep just a @@ -167,7 +202,7 @@ function ChartCard({ * explicit mapping falls back to line, which is the most forgiving * renderer (single variable + single place is always valid). */ -function getDcComponentTag( +export function getDcComponentTag( viz: ChartItem["vizType"], ): DataCommonsComponentTagName { switch (viz) { @@ -177,10 +212,6 @@ function getDcComponentTag( return "datacommons-map"; case "ranking": return "datacommons-ranking"; - case "pie": - return "datacommons-pie"; - case "gauge": - return "datacommons-gauge"; case "highlight": return "datacommons-highlight"; case "scatter": @@ -191,23 +222,38 @@ function getDcComponentTag( } } +/** + * The chart kinds whose web component reads the per-capita variable list from + * `perCapita`. Scatter reads it from `usePerCapita`. + */ +const PER_CAPITA_KINDS: ReadonlySet = new Set([ + "bar", + "line", + "map", + "ranking", + "highlight", +]); + /** * Builds the attribute set the DC component expects. Attribute names are * case-sensitive for some components (e.g. parentPlace, childPlaceType) * and the DC components accept space-separated strings for list-shaped - * attributes (variables, places). + * attributes (variables, places, perCapita, facetIds). A boolean attribute is + * set as a bare attribute when true and left off when false, which is how the + * components' boolean converter reads it. */ -function getDcComponentAttributes( +export function getDcComponentAttributes( chart: ChartItem, -): Record { - const attrs: Record = { +): DataCommonsComponentAttributes { + const variableDcids = chart.variableDcids; + const attrs: DataCommonsComponentAttributes = { header: chart.title ?? "", - variables: chart.variableDcids.join(" "), + variables: variableDcids.join(" "), }; if (chart.placeDcids && chart.placeDcids.length > 0) { // Some DC components key on `places`, others on the singular `place` - // (highlight, gauge). Emit both so the component picks the one it - // wants and ignores the other. + // (highlight). Emit both so the component reads the one it uses and + // ignores the other. attrs.places = chart.placeDcids.join(" "); attrs.place = chart.placeDcids[0]; } @@ -215,9 +261,45 @@ function getDcComponentAttributes( if (chart.childPlaceType) attrs.childPlaceType = chart.childPlaceType; if (chart.date) attrs.date = chart.date; if (chart.unit) attrs.unit = chart.unit; - // Highlight/gauge require singular `variable` not `variables`. - if (chart.variableDcids.length === 1) { - attrs.variable = chart.variableDcids[0]; + // Highlight requires singular `variable` rather than `variables`. + if (variableDcids.length === 1) { + attrs.variable = variableDcids[0]; + } + if (chart.perCapita && variableDcids.length > 0) { + if (chart.vizType === "scatter") { + attrs.usePerCapita = variableDcids.join(" "); + } else if (PER_CAPITA_KINDS.has(chart.vizType)) { + attrs.perCapita = variableDcids.join(" "); + } + } + if (chart.facetIds && variableDcids.length > 0) { + const facets = chart.facetIds; + if (chart.vizType === "highlight") { + const facetId = facets[variableDcids[0]]; + if (facetId) { + attrs.facetId = facetId; + } + } else if ( + variableDcids.every((variable) => Boolean(facets[variable])) + ) { + attrs.facetIds = variableDcids + .map((variable) => facets[variable]) + .join(" "); + } else if (variableDcids.some((variable) => Boolean(facets[variable]))) { + attrs.facetMapping = JSON.stringify(facets); + } + } + if (chart.rankingCount !== undefined) { + attrs.rankingCount = chart.rankingCount; + } + if (chart.showHighestLowest) { + attrs.showHighestLowest = true; + } + if (chart.showLowest) { + attrs.showLowest = true; + } + if (chart.sort) { + attrs.sort = chart.sort; } return attrs; } diff --git a/narratives/ui/src/hooks/chat_session_context.test.tsx b/narratives/ui/src/hooks/chat_session_context.test.tsx index 081e2226d..b849508b3 100644 --- a/narratives/ui/src/hooks/chat_session_context.test.tsx +++ b/narratives/ui/src/hooks/chat_session_context.test.tsx @@ -203,7 +203,9 @@ describe('ChatSessionProvider', () => { await act(async () => { const answered = result.current.send('What is the population of Spain?'); - next.push('content', { text: 'About 48 million.' }); + next.push('content', { + block: { index: 0, type: 'text', markdown: 'About 48 million.' }, + }); next.push('terminal', { state: 'complete', idempotency_key: 'key-2' }); next.end(); await answered; @@ -244,7 +246,9 @@ describe('ChatSessionProvider', () => { await act(async () => { const answered = result.current.send('What is the population of Spain?'); - second.push('content', { text: 'About 48 million.' }); + second.push('content', { + block: { index: 0, type: 'text', markdown: 'About 48 million.' }, + }); second.push('terminal', { state: 'complete', idempotency_key: 'key-2' }); second.end(); await answered; diff --git a/narratives/ui/src/hooks/use_sse_chat.test.ts b/narratives/ui/src/hooks/use_sse_chat.test.ts index ac840fe23..574ef1002 100644 --- a/narratives/ui/src/hooks/use_sse_chat.test.ts +++ b/narratives/ui/src/hooks/use_sse_chat.test.ts @@ -14,13 +14,18 @@ import { applyEvent, applySignedWindow, assignCitationNumbers, + getTurnCharts, + hasAnswer, parseSseStream, transcriptRequestFields, useSseChat, + type AnswerBlock, + type ChartItem, type ChatStreamEvent, type ChatTurn, type ProvenanceItem, type ConversationStateSlots, + type RawAnswerBlock, type TurnStatus, type WireTurn, } from "./use_sse_chat"; @@ -61,6 +66,22 @@ function frame(event: string, data: unknown, id?: number): string { return `${idLine}event: ${event}\r\ndata: ${JSON.stringify(data)}\r\n\r\n`; } +/** Builds the `content` payload of one text block. */ +function buildTextBlock(markdown: string): { block: RawAnswerBlock } { + return { block: { type: "text", markdown } }; +} + +/** + * Builds a `content` event carrying one chart block. The chart is untyped so + * a test can send a payload that violates the wire contract. + */ +function buildChartEvent(chart: Record): ChatStreamEvent { + return { + event: "content", + data: { block: { type: "chart", chart } }, + } as unknown as ChatStreamEvent; +} + /** Returns a byte stream that yields each chunk as one read. */ function streamOf(chunks: string[]): ReadableStream { const encoder = new TextEncoder(); @@ -128,6 +149,7 @@ const blankTurn = (): ChatTurn => ({ toolCalls: [], thoughts: [], text: "", + blocks: [], provenance: [], }); @@ -259,7 +281,7 @@ describe("parseSseStream", () => { { phase: "discovery", message: "Querying" }, 1, ); - const text = frame("content", { text: "About 68 million." }, 2); + const text = frame("content", buildTextBlock("About 68 million."), 2); const splitAt = status.length - 1; const events = await parseAll([ @@ -269,7 +291,7 @@ describe("parseSseStream", () => { expect(events).toEqual([ { event: "status", data: { phase: "discovery", message: "Querying" } }, - { event: "content", data: { text: "About 68 million." } }, + { event: "content", data: buildTextBlock("About 68 million.") }, ]); }); @@ -281,7 +303,7 @@ describe("parseSseStream", () => { // Expectation: All three events are decoded with their names and // payloads, and no spurious frames are produced. const status = `event: status\rdata: ${JSON.stringify({ phase: "discovery" })}\r\r`; - const text = frame("content", { text: "About 68 million." }, 2); + const text = frame("content", buildTextBlock("About 68 million."), 2); const complete = { state: "complete", ...TERMINAL_FIELDS }; const terminal = frame("terminal", complete, 3); @@ -289,7 +311,7 @@ describe("parseSseStream", () => { expect(events).toEqual([ { event: "status", data: { phase: "discovery" } }, - { event: "content", data: { text: "About 68 million." } }, + { event: "content", data: buildTextBlock("About 68 million.") }, { event: "terminal", data: complete }, ]); }); @@ -330,7 +352,9 @@ describe("parseSseStream", () => { controller.enqueue( encoder.encode(frame("status", { phase: "discovery" })), ); - controller.enqueue(encoder.encode(frame("content", { text: "x" }))); + controller.enqueue( + encoder.encode(frame("content", buildTextBlock("x"))), + ); }, cancel() { canceled = true; @@ -368,16 +392,41 @@ describe("parseSseStream", () => { describe("applyEvent", () => { it("builds the turn from status, thought, and content events", () => { // Test: Reduction of the non-terminal events. - // Situation: A turn receives a status, a thought, two text chunks, - // duplicate sources, a truncation flag, and a chart config. - // Expectation: The status matches the phase, the text chunks are - // concatenated, duplicate source URLs are removed, truncation is set, - // and the chart config is mapped to camelCase. + // Situation: A turn receives a status, a thought, a text block, a chart + // block, a second text block, duplicate sources, and a truncation + // flag. + // Expectation: The status matches the phase, the blocks are kept in + // order with the chart mapped to camelCase, `text` is the text blocks + // joined by a blank line (the string the agent signs), duplicate + // source URLs are removed, and truncation is set. const events: ChatStreamEvent[] = [ { event: "status", data: { phase: "synthesis" } }, { event: "thought", data: { thought: "Checking", phase: "discovery" } }, - { event: "content", data: { text: "About " } }, - { event: "content", data: { text: "68 million." } }, + { event: "content", data: buildTextBlock("About 68 million.") }, + { + event: "content", + data: { + block: { + type: "chart", + chart: { + viz_type: "ranking", + title: "GDP per capita by state", + variable_dcids: ["Amount_EconomicActivity_GrossDomesticProduct_Nominal"], + parent_place: "country/USA", + child_place_type: "State", + date: "2023", + per_capita: true, + facet_ids: { + Amount_EconomicActivity_GrossDomesticProduct_Nominal: "f1", + }, + ranking_count: 5, + show_highest_lowest: true, + show_lowest: false, + }, + }, + }, + }, + { event: "content", data: buildTextBlock("It grew 0.3% last year.") }, { event: "content", data: { @@ -388,53 +437,211 @@ describe("applyEvent", () => { }, }, { event: "content", data: { data_status: { truncated: true } } }, - { - event: "content", - data: { chart_config: { should_render: true, hide_charts: true } }, - }, ]; const turn = events.reduce(applyEvent, blankTurn()); expect(turn.status).toBe("synthesis"); expect(turn.thoughts).toEqual([{ text: "Checking", phase: "discovery" }]); - expect(turn.text).toBe("About 68 million."); + const expectedBlocks: AnswerBlock[] = [ + { index: 0, type: "text", markdown: "About 68 million." }, + { + index: 1, + type: "chart", + chart: { + vizType: "ranking", + title: "GDP per capita by state", + variableDcids: ["Amount_EconomicActivity_GrossDomesticProduct_Nominal"], + placeDcids: undefined, + parentPlace: "country/USA", + childPlaceType: "State", + date: "2023", + unit: undefined, + perCapita: true, + facetIds: { + Amount_EconomicActivity_GrossDomesticProduct_Nominal: "f1", + }, + rankingCount: 5, + showHighestLowest: true, + showLowest: false, + sort: undefined, + }, + }, + { index: 2, type: "text", markdown: "It grew 0.3% last year." }, + ]; + expect(turn.blocks).toEqual(expectedBlocks); + expect(turn.text).toBe("About 68 million.\n\nIt grew 0.3% last year."); expect(turn.provenance).toEqual([{ name: "INSEE", url: "https://insee.fr/" }]); expect(turn.truncated).toBe(true); - expect(turn.chartConfig).toEqual({ - shouldRender: true, - hideCharts: true, - charts: undefined, - }); }); - it("marks the text complete when the chart phase starts", () => { - // Test: End-of-text signal. - // Situation: Text streams, then the agent announces the chart phase. - // Expectation: The text is incomplete while it streams and complete once - // the chart phase starts, without leaving the synthesis status. - const streaming = applyEvent( - { ...blankTurn(), status: "synthesis" }, - { event: "content", data: { text: "About 68 million." } }, + it("ignores a block without content", () => { + // Test: Tolerance of malformed block frames. + // Situation: A turn receives an empty text block, chart blocks whose + // chart is missing or lacks variables, an unrecognized block type, and + // then a well-formed text block. + // Expectation: Only the well-formed block is kept, numbered by its + // position, and `text` holds its markdown alone. + const events = [ + { event: "content", data: buildTextBlock("") }, + { event: "content", data: { block: { type: "chart" } } }, + { + event: "content", + data: { + block: { + type: "chart", + chart: { variable_dcids: ["Count_Person"] }, + }, + }, + }, + { + event: "content", + data: { + block: { + type: "chart", + chart: { viz_type: "line", variable_dcids: [] }, + }, + }, + }, + { + event: "content", + data: { block: { type: "unknown", markdown: "Ignored." } }, + }, + { event: "content", data: { block: { type: "text", markdown: "Only this." } } }, + ] as unknown as ChatStreamEvent[]; + + const turn = events.reduce(applyEvent, blankTurn()); + + expect(turn.blocks).toEqual([{ index: 0, type: "text", markdown: "Only this." }]); + expect(turn.text).toBe("Only this."); + }); + + it("drops a chart block of a kind the agent does not emit", () => { + // Test: Chart kind validation at the boundary. + // Situation: A turn receives chart blocks whose `viz_type` is `slider`, + // which the UI type lists but the agent never emits, and `pie`. + // Expectation: Neither block is kept. + const turn = [ + buildChartEvent({ viz_type: "slider", variable_dcids: ["Count_Person"] }), + buildChartEvent({ viz_type: "pie", variable_dcids: ["Count_Person"] }), + ].reduce(applyEvent, blankTurn()); + + expect(turn.blocks).toEqual([]); + }); + + it("drops a chart block whose variables are not all non-empty strings", () => { + // Test: Variable validation at the boundary. + // Situation: A turn receives chart blocks whose `variable_dcids` hold an + // empty string, a number, and a valid DCID beside an empty string. + // Expectation: None of the blocks is kept. + const turn = [ + buildChartEvent({ viz_type: "line", variable_dcids: [""] }), + buildChartEvent({ viz_type: "line", variable_dcids: [42] }), + buildChartEvent({ viz_type: "line", variable_dcids: ["Count_Person", ""] }), + ].reduce(applyEvent, blankTurn()); + + expect(turn.blocks).toEqual([]); + }); + + it("maps a bar chart's sort order from the wire", () => { + // Test: Mapping of `sort` at the boundary. + // Situation: A bar chart block with `sort: "descending"`. + // Expectation: The chart the turn renders carries `sort: "descending"`. + const turn = applyEvent( + blankTurn(), + buildChartEvent({ + viz_type: "bar", + variable_dcids: ["Count_Person"], + place_dcids: ["country/FRA", "country/DEU"], + sort: "descending", + }), ); - const finished = applyEvent(streaming, { - event: "status", - data: { phase: "chart_config" }, - }); - expect(streaming.textComplete).toBeUndefined(); - expect(finished.textComplete).toBe(true); - expect(finished.status).toBe("synthesis"); + expect(getTurnCharts(turn)[0].sort).toBe("descending"); + }); + + it("keeps only the string-valued entries of a chart's facet ids", () => { + // Test: Facet mapping validation at the boundary. + // Situation: Chart blocks whose `facet_ids` is an array, an object with + // one string and one numeric value, and an object with only a numeric + // value. + // Expectation: The array and the all-numeric object yield no facet ids, + // and the mixed object keeps only its string entry. + const turn = [ + buildChartEvent({ + viz_type: "line", + variable_dcids: ["Count_Person"], + facet_ids: ["f1"], + }), + buildChartEvent({ + viz_type: "line", + variable_dcids: ["Count_Person"], + facet_ids: { Count_Person: "f1", Median_Age_Person: 2 }, + }), + buildChartEvent({ + viz_type: "line", + variable_dcids: ["Count_Person"], + facet_ids: { Count_Person: 2 }, + }), + ].reduce(applyEvent, blankTurn()); + + expect(getTurnCharts(turn).map((chart) => chart.facetIds)).toEqual([ + undefined, + { Count_Person: "f1" }, + undefined, + ]); + }); + + it("numbers blocks by arrival even when a malformed block is dropped", () => { + // Test: Client-assigned block positions. + // Situation: A text block, a chart block of an unknown kind, a valid + // chart block, and a second text block arrive, each carrying a wire + // `index` that does not match its arrival position. + // Expectation: The three kept blocks are numbered 0, 1, and 2 in arrival + // order, so their React keys stay unique and sequential. + const events = [ + { event: "content", data: { block: { index: 5, type: "text", markdown: "One." } } }, + { + event: "content", + data: { + block: { + index: 5, + type: "chart", + chart: { viz_type: "pie", variable_dcids: ["Count_Person"] }, + }, + }, + }, + { + event: "content", + data: { + block: { + index: 9, + type: "chart", + chart: { viz_type: "bar", variable_dcids: ["Count_Person"] }, + }, + }, + }, + { event: "content", data: { block: { index: 1, type: "text", markdown: "Two." } } }, + ] as unknown as ChatStreamEvent[]; + + const turn = events.reduce(applyEvent, blankTurn()); + + expect(turn.blocks.map((block) => [block.index, block.type])).toEqual([ + [0, "text"], + [1, "chart"], + [2, "text"], + ]); }); it("moves the turn through the discovery and processing phases", () => { // Test: The status phases of the discovery loop. // Situation: A turn receives a `discovery` status, a thought tagged // `discovery`, a `processing` status, a `discovery` status again as - // the loop retries, and a `chart_config` status. + // the loop retries, and a status whose phase this build does not + // recognize. // Expectation: Each of the first four statuses becomes the turn status - // in order, the thought keeps its `discovery` phase, and the - // `chart_config` status leaves the status where it was. + // in order, the thought keeps its `discovery` phase, and the unknown + // phase leaves the status where it was. const statuses: TurnStatus[] = []; let turn = blankTurn(); const events: ChatStreamEvent[] = [ @@ -442,7 +649,7 @@ describe("applyEvent", () => { { event: "thought", data: { thought: "Fetching population", phase: "discovery" } }, { event: "status", data: { phase: "processing", message: "Analyzing data..." } }, { event: "status", data: { phase: "discovery", message: "Querying data tools..." } }, - { event: "status", data: { phase: "chart_config" } }, + { event: "status", data: { phase: "later_phase" } } as unknown as ChatStreamEvent, ]; for (const event of events) { turn = applyEvent(turn, event); @@ -595,7 +802,7 @@ describe("useSseChat", () => { status: 200, statusText: "OK", body: streamOf([ - frame("content", { text: "About 68 million." }, 1), + frame("content", buildTextBlock("About 68 million."), 1), frame( "terminal", { @@ -703,19 +910,19 @@ describe("useSseChat", () => { it("marks a stream that ends without a terminal event as interrupted", async () => { // Test: EOF without a terminal event. - // Situation: The stream delivers a status event and partial text, and + // Situation: The stream delivers a status event and one text block, and // then ends without a terminal event. - // Expectation: The turn keeps its partial text but ends as an error + // Expectation: The turn keeps the text it received but ends as an error // with the interrupted-connection message, not stuck mid-stream. const { result } = renderChat([ frame("status", { phase: "synthesis" }, 1), - frame("content", { text: "About 68" }, 2), + frame("content", buildTextBlock("About 68 million."), 2), ]); await act(() => result.current.chat.send("What is the population of France?")); const [turn] = result.current.turns; - expect(turn.text).toBe("About 68"); + expect(turn.text).toBe("About 68 million."); expect(turn.status).toBe("error"); expect(turn.error).toBe(INTERRUPTED_TURN_ERROR); expect(result.current.chat.error).toBe(INTERRUPTED_TURN_ERROR); @@ -728,7 +935,7 @@ describe("useSseChat", () => { // Expectation: The turn is done with its follow-ups, no error is set, // and the request carried a non-empty idempotency key. const { result, fetchMock } = renderChat([ - frame("content", { text: "About 68 million." }, 1), + frame("content", buildTextBlock("About 68 million."), 1), frame("terminal", { state: "complete", ...TERMINAL_FIELDS }, 2), frame("follow_ups", { follow_up_questions: ["And Spain?"] }, 3), ]); @@ -781,13 +988,13 @@ describe("useSseChat", () => { it("reports a stream that fails mid-turn as interrupted", async () => { // Test: A connection lost after the response started. - // Situation: The stream delivers some text, then fails with the + // Situation: The stream delivers one text block, then fails with the // browser's network error. // Expectation: The turn ends as an error with the interrupted-connection // message rather than the browser's wording. const { result } = renderChat( failingStreamOf( - [frame("content", { text: "About 68" }, 1)], + [frame("content", buildTextBlock("About 68 million."), 1)], new TypeError("network error"), ), ); @@ -810,7 +1017,7 @@ describe("useSseChat", () => { const { result } = renderChat(stream.body); await act(async () => { void result.current.chat.send("What is the population of France?"); - stream.push(frame("content", { text: "About 68 million." }, 1)); + stream.push(frame("content", buildTextBlock("About 68 million."), 1)); stream.push( frame("terminal", { state: "complete", ...TERMINAL_FIELDS }, 2), ); @@ -852,7 +1059,7 @@ describe("useSseChat", () => { }); await act(async () => { - stream.push(frame("content", { text: "About 68 million." }, 1)); + stream.push(frame("content", buildTextBlock("About 68 million."), 1)); stream.push( frame( "terminal", @@ -979,7 +1186,7 @@ describe("useSseChat", () => { // Expectation: `send` posts to `/narratives/agent/chat/stream`. vi.stubEnv("BASE_URL", "/narratives/"); const { result, fetchMock } = renderChat([ - frame("content", { text: "About 68 million." }, 1), + frame("content", buildTextBlock("About 68 million."), 1), frame("terminal", { state: "complete", ...TERMINAL_FIELDS }, 2), ]); @@ -998,6 +1205,61 @@ const WDI: ProvenanceItem = { const GHO: ProvenanceItem = { name: "GHO", url: "https://who.int/gho" }; const OECD: ProvenanceItem = { name: "OECD", url: "https://oecd.org/health" }; +const LINE_CHART: ChartItem = { + vizType: "line", + title: "Population, France", + variableDcids: ["Count_Person"], + placeDcids: ["country/FRA"], +}; + +describe("getTurnCharts", () => { + it("reads the charts from the turn's chart blocks", () => { + // Test: The chart list of a turn streamed as blocks. + // Situation: One turn holds a text block and a chart block, another + // holds only a text block, and a third holds no blocks. + // Expectation: The first returns the chart block's chart and the others + // return none. + const withChartBlock: ChatTurn = { + ...blankTurn(), + blocks: [ + { index: 0, type: "text", markdown: "About 68 million." }, + { index: 1, type: "chart", chart: LINE_CHART }, + ], + }; + const textOnlyBlocks: ChatTurn = { + ...blankTurn(), + blocks: [{ index: 0, type: "text", markdown: "About 68 million." }], + }; + + expect(getTurnCharts(withChartBlock)).toEqual([LINE_CHART]); + expect(getTurnCharts(textOnlyBlocks)).toEqual([]); + expect(getTurnCharts(blankTurn())).toEqual([]); + }); +}); + +describe("hasAnswer", () => { + it("reports only a turn with answer text as having an answer", () => { + // Test: The predicate the turn view renders the answer panel on. + // Situation: A blank turn, a turn whose only content is a chart block, + // and a turn with a text block. + // Expectation: Only the turn with answer text has an answer; the + // chart-only turn is a failed turn and has none. + const chartOnly: ChatTurn = { + ...blankTurn(), + blocks: [{ index: 0, type: "chart", chart: LINE_CHART }], + }; + const withText: ChatTurn = { + ...blankTurn(), + text: "About 68 million.", + blocks: [{ index: 0, type: "text", markdown: "About 68 million." }], + }; + + expect(hasAnswer(blankTurn())).toBe(false); + expect(hasAnswer(chartOnly)).toBe(false); + expect(hasAnswer(withText)).toBe(true); + }); +}); + function citedTurn( provenance: ProvenanceItem[], extra: Partial = {}, @@ -1008,6 +1270,7 @@ function citedTurn( toolCalls: [], thoughts: [], text: "a", + blocks: [{ index: 0, type: "text", markdown: "a" }], provenance, ...extra, }; @@ -1021,6 +1284,22 @@ describe("assignCitationNumbers", () => { expect(assignCitationNumbers([citedTurn([WDI, GHO])])).toEqual([[1, 2]]); }); + it("gives no numbers to a turn whose answer consists solely of a chart block", () => { + // Test: Citation numbering for a block-streamed turn with no prose text. + // Situation: A turn holds a chart block and provenance while `text` is + // empty, followed by an answer with text. + // Expectation: The chart-only turn takes no numbers, so the next answer + // opens at [1]. + const chartOnly = citedTurn([WDI, GHO], { + text: "", + blocks: [{ index: 0, type: "chart", chart: LINE_CHART }], + }); + expect(assignCitationNumbers([chartOnly, citedTurn([OECD])])).toEqual([ + [], + [1], + ]); + }); + it("keeps counting into the next answer", () => { // Test: The reported bug. // Situation: A second answer citing a source the first did not. @@ -1058,7 +1337,11 @@ describe("assignCitationNumbers", () => { // Expectation: It takes no numbers, so the next answer still opens at [1] // rather than at [3] with [1] and [2] nowhere on the page. const stopped = citedTurn([WDI, GHO], { stopped: true }); - const errored = citedTurn([OECD], { status: "error", text: "" }); + const errored = citedTurn([OECD], { + status: "error", + text: "", + blocks: [], + }); expect(assignCitationNumbers([stopped, citedTurn([OECD])])).toEqual([ [], [1], diff --git a/narratives/ui/src/hooks/use_sse_chat.ts b/narratives/ui/src/hooks/use_sse_chat.ts index 76539b9b5..9fa477d7b 100644 --- a/narratives/ui/src/hooks/use_sse_chat.ts +++ b/narratives/ui/src/hooks/use_sse_chat.ts @@ -41,9 +41,8 @@ export interface ThoughtEvent { } /** - * One chart the agent asks the UI to render, in the UI's camelCase domain - * shape. Produced by {@link mapRawChartItem} from the snake_case - * {@link RawChartItem} wire format. + * One chart to render in the UI's camelCase domain shape, produced by + * {@link mapRawChartItem} from the snake_case {@link RawChartItem} wire format. */ export interface ChartItem { vizType: @@ -51,9 +50,7 @@ export interface ChartItem { | "bar" | "map" | "ranking" - | "pie" | "highlight" - | "gauge" | "scatter" | "slider"; title?: string; @@ -63,14 +60,23 @@ export interface ChartItem { childPlaceType?: string; date?: string; unit?: string; + /** Divide each variable by population in the Data Commons web component. */ + perCapita?: boolean; + /** Maps each charted variable DCID to the facet ID that served its table. */ + facetIds?: Record; + /** How many places a ranking lists from each end. */ + rankingCount?: number; + /** Show both the highest and the lowest places of a ranking. */ + showHighestLowest?: boolean; + /** Show the lowest places of a ranking instead of the highest. */ + showLowest?: boolean; + /** Sort order of a bar chart's places. */ + sort?: "ascending" | "descending"; } /** - * The agent's chart-rendering directive for a turn, in camelCase domain shape. - * Produced by {@link mapRawChartConfig} from the snake_case - * {@link RawChartConfig} wire format. Supports the modern multi-chart format - * (`charts: [...]`); legacy single-chart fallback fields are folded into - * `charts` during mapping. + * The chart list a `ChartTile` renders. The tile renders nothing unless + * `shouldRender` is true and `hideCharts` is unset. */ export interface ChartConfig { shouldRender: boolean; @@ -79,12 +85,10 @@ export interface ChartConfig { } /** - * Wire format of one chart item as emitted by the agent - * (the agent's `gemini/schemas.py` CHART_CONFIG_SCHEMA + homepage.html - * renderDCComponent). - * snake_case matches the SSE payload; {@link mapRawChartItem} converts to the - * camelCase {@link ChartItem} the UI consumes, so an upstream schema rename - * only requires updating the mapper. + * Wire format of one chart serialized by `workflows/charts.py` inside a + * chart block. `snake_case` matches the SSE payload; {@link mapRawChartItem} + * converts to the camelCase {@link ChartItem} the UI consumes, so an upstream + * rename only requires updating the mapper. */ interface RawChartItem { viz_type: ChartItem["vizType"]; @@ -95,25 +99,40 @@ interface RawChartItem { child_place_type?: string; date?: string; unit?: string; + per_capita?: boolean; + facet_ids?: Record; + ranking_count?: number; + show_highest_lowest?: boolean; + show_lowest?: boolean; + sort?: ChartItem["sort"]; } /** - * Wire format of the chart_config SSE field. Older agent versions emit the - * legacy single-chart fields (`viz_type`, `variable_dcids`, …) at the top - * level instead of inside a `charts` array; {@link mapRawChartConfig} - * normalizes both shapes. + * The chart kinds `serialize_resolved_chart` in `workflows/charts.py` emits. + * A chart block of any other kind is dropped. */ -interface RawChartConfig { - should_render: boolean; - hide_charts?: boolean; - charts?: RawChartItem[]; - viz_type?: ChartItem["vizType"]; - title?: string; - variable_dcids?: string[]; - place_dcids?: string[]; - parent_place?: string; - child_place_type?: string; - date?: string; +const CHART_VIZ_TYPES: ReadonlySet = new Set([ + "line", + "bar", + "map", + "ranking", + "scatter", + "highlight", +]); + +/** + * Reads a chart's facet mapping from the wire, keeping only the string-valued + * entries of a plain object. Returns undefined when no entry remains, so a + * malformed mapping is treated as absent. + */ +function readFacetIds(value: unknown): Record | undefined { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return undefined; + } + const entries = Object.entries(value).filter( + (entry): entry is [string, string] => typeof entry[1] === "string", + ); + return entries.length > 0 ? Object.fromEntries(entries) : undefined; } /** Converts one raw snake_case chart item into the camelCase domain shape. */ @@ -121,40 +140,53 @@ function mapRawChartItem(raw: RawChartItem): ChartItem { return { vizType: raw.viz_type, title: raw.title, - variableDcids: raw.variable_dcids ?? [], + variableDcids: raw.variable_dcids, placeDcids: raw.place_dcids, parentPlace: raw.parent_place, childPlaceType: raw.child_place_type, date: raw.date, unit: raw.unit, + perCapita: raw.per_capita, + facetIds: readFacetIds(raw.facet_ids), + rankingCount: raw.ranking_count, + showHighestLowest: raw.show_highest_lowest, + showLowest: raw.show_lowest, + sort: raw.sort, }; } /** - * Converts the raw chart_config payload into the camelCase domain shape, - * folding the legacy top-level single-chart fields into a one-element - * `charts` array when no modern `charts` list is present. + * Wire format of one streamed answer block: a completed Markdown paragraph, + * or a chart resolved against the turn's cached tables. */ -function mapRawChartConfig(raw: RawChartConfig): ChartConfig { - let charts = raw.charts?.map(mapRawChartItem); - if ((!charts || charts.length === 0) && raw.variable_dcids?.length) { - charts = [ - mapRawChartItem({ - viz_type: raw.viz_type ?? "line", - title: raw.title, - variable_dcids: raw.variable_dcids, - place_dcids: raw.place_dcids, - parent_place: raw.parent_place, - child_place_type: raw.child_place_type, - date: raw.date, - }), - ]; - } - return { - shouldRender: raw.should_render, - hideCharts: raw.hide_charts, - charts, - }; +export type RawAnswerBlock = + | { type: "text"; markdown: string } + | { type: "chart"; chart: RawChartItem }; + +/** + * One block of an answer in the UI's domain shape, rendered in order. `index` + * is the block's position in the turn, which the answer panel uses as its key. + * The position is assigned on arrival, so a dropped malformed block cannot + * produce duplicate or invalid React keys. + */ +export type AnswerBlock = + | { index: number; type: "text"; markdown: string } + | { index: number; type: "chart"; chart: ChartItem }; + +/** Returns the charts a turn renders: the charts of its chart blocks, in order. */ +export function getTurnCharts(turn: ChatTurn): ChartItem[] { + return turn.blocks.flatMap((block) => + block.type === "chart" ? [block.chart] : [], + ); +} + +/** + * Reports whether a turn has an answer to show. The answer panel is shown + * only once a turn has answer text: a turn with charts but no prose is a + * failed turn. + */ +export function hasAnswer(turn: ChatTurn): boolean { + return turn.text !== ""; } /** A source attribution (display name + link) surfaced with an answer. */ @@ -250,14 +282,13 @@ export interface ChatTurn { status: TurnStatus; toolCalls: ToolCallEvent[]; thoughts: ThoughtEvent[]; - text: string; /** - * Set once the answer text has finished streaming: on the first event the - * agent sends after synthesis, which is the chart phase's status or, when - * there is no chart to prepare, the chart config itself. + * The answer's prose: the text blocks joined by blank lines, which is the + * string the agent signs. Chart blocks are not part of it. */ - textComplete?: boolean; - chartConfig?: ChartConfig; + text: string; + /** The answer's text and chart blocks in emission order. */ + blocks: AnswerBlock[]; provenance: ProvenanceItem[]; /** Holds self-contained follow-up questions emitted by the agent after the terminal event. */ followUps?: string[]; @@ -302,7 +333,7 @@ type TerminalState = "complete" | "error" | "refused" | "canceled"; * per frame. */ interface ContentPayload { - text?: string; + block?: RawAnswerBlock; tool_call?: ToolCallEvent; sources?: ProvenanceItem[]; /** @@ -310,7 +341,6 @@ interface ContentPayload { * the flag the UI acts on. */ data_status?: { truncated?: boolean; has_data?: boolean }; - chart_config?: RawChartConfig; } /** Identifies one turn of the signed window listed in a `terminal` event. */ @@ -358,12 +388,7 @@ export type ChatStreamEvent = | { event: "status"; data: { - phase: - | "classify" - | "discovery" - | "processing" - | "synthesis" - | "chart_config"; + phase: "classify" | "discovery" | "processing" | "synthesis"; message?: string; }; } @@ -388,10 +413,14 @@ const newTurn = (userMessage: string, idempotencyKey: string): ChatTurn => ({ toolCalls: [], thoughts: [], text: "", + blocks: [], provenance: [], idempotencyKey, }); +/** The separator between text blocks in the signed answer text. */ +const TEXT_BLOCK_SEPARATOR = "\n\n"; + /** Represents a turn that carries everything needed to send it back as signed context. */ type SignedTurn = ChatTurn & { idempotencyKey: string; @@ -412,27 +441,25 @@ function isSignedTurn(turn: ChatTurn): turn is SignedTurn { ); } -/** - * Builds the transcript fields of a request from the signed turns. The query - * and answer are sent exactly as stored, because the agent verifies them - * against the turn's signature. - */ /** * Labels every answer's citations so one number means one source thread-wide. * * A source keeps the number it was first given; anything new takes the next * free one. Returns one array per turn, parallel to that turn's `provenance`. * - * A turn that renders no answer -- stopped, or errored before any text -- - * takes no numbers, matching the condition `TurnView` renders `AnswerPanel` - * on. Numbering one would retire `[1]` and `[2]` to a turn with no Citations - * list, and the next answer would open at `[3]`. + * A turn that renders no answer (stopped, or with no answer text, such as + * one that errored before any text) takes no numbers, matching the condition + * `TurnView` renders `AnswerPanel` on. Numbering one would retire `[1]` and + * `[2]` to a turn with no Citations list, and the next answer would open at + * `[3]`. */ export function assignCitationNumbers(turns: ChatTurn[]): number[][] { const assigned = new Map(); let next = 1; return turns.map((turn) => { - if (!turn.text || turn.stopped) return []; + if (!hasAnswer(turn) || turn.stopped) { + return []; + } return turn.provenance.map((source) => { const key = source.url || source.name; const existing = assigned.get(key); @@ -443,6 +470,11 @@ export function assignCitationNumbers(turns: ChatTurn[]): number[][] { }); } +/** + * Builds the transcript fields of a request from the signed turns. The query + * and answer are sent exactly as stored, because the agent verifies them + * against the turn's signature. + */ export function transcriptRequestFields(turns: ChatTurn[]): { turns: WireTurn[]; compacted_summary: string | null; @@ -856,11 +888,58 @@ function applyTerminal(turn: ChatTurn, terminal: TerminalPayload): ChatTurn { return { ...turn, status: "error", error: terminalError(terminal) }; } +/** + * Appends one answer block to the turn at position `blocks.length`. A text + * block also extends `text`, the signed answer, with the same separator the + * agent joins its text blocks by, so the two stay byte for byte equal. + * Malformed blocks are ignored: unrecognized types, empty text blocks, and + * chart blocks whose kind is not one the agent emits or whose variables are + * missing or not all non-empty strings. + */ +function applyBlock(turn: ChatTurn, raw: RawAnswerBlock): ChatTurn { + const blocks = turn.blocks; + const index = blocks.length; + if (raw.type === "chart") { + if ( + typeof raw.chart !== "object" || + raw.chart === null || + !CHART_VIZ_TYPES.has(raw.chart.viz_type) || + !Array.isArray(raw.chart.variable_dcids) || + raw.chart.variable_dcids.length === 0 || + !raw.chart.variable_dcids.every( + (dcid: unknown) => typeof dcid === "string" && dcid.length > 0, + ) + ) { + console.warn("Ignoring a malformed chart block."); + return turn; + } + const block: AnswerBlock = { + index, + type: "chart", + chart: mapRawChartItem(raw.chart), + }; + return { ...turn, blocks: [...blocks, block] }; + } + if ( + raw.type !== "text" || + typeof raw.markdown !== "string" || + raw.markdown.length === 0 + ) { + console.warn("Ignoring a malformed or empty answer block."); + return turn; + } + const block: AnswerBlock = { index, type: "text", markdown: raw.markdown }; + const text = turn.text + ? turn.text + TEXT_BLOCK_SEPARATOR + raw.markdown + : raw.markdown; + return { ...turn, blocks: [...blocks, block], text }; +} + /** Applies one `content` payload to the turn state. */ function applyContent(turn: ChatTurn, content: ContentPayload): ChatTurn { let next = turn; - if (typeof content.text === "string") { - next = { ...next, text: next.text + content.text }; + if (content.block && typeof content.block === "object") { + next = applyBlock(next, content.block); } if (content.tool_call) { next = { ...next, toolCalls: [...next.toolCalls, content.tool_call] }; @@ -881,13 +960,6 @@ function applyContent(turn: ChatTurn, content: ContentPayload): ChatTurn { if (content.data_status?.truncated) { next = { ...next, truncated: true }; } - if (content.chart_config) { - next = { - ...next, - textComplete: true, - chartConfig: mapRawChartConfig(content.chart_config), - }; - } return next; } @@ -903,8 +975,7 @@ export function applyEvent(turn: ChatTurn, evt: ChatStreamEvent): ChatTurn { ) { return { ...turn, status: evt.data.phase }; } - // The chart phase only starts once synthesis has finished streaming. - return { ...turn, textComplete: true }; + return turn; case "thought": return { ...turn, diff --git a/narratives/ui/src/types/data-commons-elements.d.ts b/narratives/ui/src/types/data-commons-elements.d.ts index 8490dc2a7..d72a40514 100644 --- a/narratives/ui/src/types/data-commons-elements.d.ts +++ b/narratives/ui/src/types/data-commons-elements.d.ts @@ -23,6 +23,7 @@ interface DataCommonsAttributes header?: string; date?: string; colors?: string; + unit?: string; // Plural (line / bar / pie / scatter / etc.) variables?: string; @@ -36,6 +37,17 @@ interface DataCommonsAttributes parentPlace?: string; childPlaceType?: string; + // Per-capita normalization: a space-separated list of the variable DCIDs + // to divide by population (bar / line / map / ranking / highlight) + perCapita?: string; + + // Source facet pinning: `facetId` on single-variable tiles such as + // highlight, `facetIds` as a space-separated list aligned with `variables`, + // and `facetMapping` as a JSON object from variable DCID to facet ID + facetId?: string; + facetIds?: string; + facetMapping?: string; + // Bar sort?: 'ascending' | 'descending'; @@ -48,6 +60,7 @@ interface DataCommonsAttributes // Ranking rankingCount?: string | number; showHighestLowest?: boolean | '' | 'true' | 'false'; + showLowest?: boolean | '' | 'true' | 'false'; // Gauge min?: string | number; @@ -55,6 +68,8 @@ interface DataCommonsAttributes // Scatter showPlaceLabels?: boolean | '' | 'true' | 'false'; + // Scatter's name for the per-capita variable list + usePerCapita?: string; } declare module 'react' { diff --git a/narratives/ui/src/utils/citation_marker.test.ts b/narratives/ui/src/utils/citation_marker.test.ts new file mode 100644 index 000000000..560180977 --- /dev/null +++ b/narratives/ui/src/utils/citation_marker.test.ts @@ -0,0 +1,79 @@ +/** + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +/** @fileoverview Tests citation marker matching and number parsing. */ + +import { describe, expect, it } from 'vitest'; +import { + createCitationMarkerPattern, + parseCitationNumbers, +} from './citation_marker'; + +/** Parses the numbers of every citation marker in `text`, one array per marker. */ +function parseMarkers(text: string): number[][] { + return [...text.matchAll(createCitationMarkerPattern())].map((match) => + parseCitationNumbers(match[1]), + ); +} + +describe('createCitationMarkerPattern', () => { + it('accepts single and composite markers', () => { + // Test: Single and composite citation markers. + // Situation: A single marker, a composite marker with a space, a composite + // marker without a space, and two markers with one- and two-digit numbers. + // Expectation: Each marker is matched and its numbers are parsed in order. + expect(parseMarkers('[1]')).toEqual([[1]]); + expect(parseMarkers('[1, 2]')).toEqual([[1, 2]]); + expect(parseMarkers('[1,2]')).toEqual([[1, 2]]); + expect(parseMarkers('[12, 3] and [99]')).toEqual([[12, 3], [99]]); + }); + + it('rejects brackets that are not a marker', () => { + // Test: Boundaries of the marker syntax. + // Situation: A three-digit number, a trailing comma, a letter, a + // space-padded number, and empty brackets. + // Expectation: None matches as a citation marker. + for (const text of ['[123]', '[1,]', '[1, b]', '[ 1 ]', '[]']) { + expect(parseMarkers(text)).toEqual([]); + } + }); + + it('returns a fresh pattern on every call', () => { + // Test: Isolation of the global pattern's state. + // Situation: One pattern is advanced by a match before another is + // requested. + // Expectation: The second pattern starts from index 0 of its input, so one + // caller's `lastIndex` cannot skip another caller's marker. + const first = createCitationMarkerPattern(); + first.exec('[1] [2]'); + + const second = createCitationMarkerPattern(); + + expect(second).not.toBe(first); + expect(second.lastIndex).toBe(0); + expect(second.exec('[1] [2]')?.index).toBe(0); + }); +}); + +describe('parseCitationNumbers', () => { + it('drops repeated numbers in order of first occurrence', () => { + // Test: A marker that repeats a source. + // Situation: The capture groups of `[1, 1]` and `[2, 1, 2]`. + // Expectation: Each number appears once, in the order it first appears. + expect(parseCitationNumbers('1, 1')).toEqual([1]); + expect(parseCitationNumbers('2, 1, 2')).toEqual([2, 1]); + }); +}); diff --git a/narratives/ui/src/utils/citation_marker.ts b/narratives/ui/src/utils/citation_marker.ts new file mode 100644 index 000000000..9d19614ba --- /dev/null +++ b/narratives/ui/src/utils/citation_marker.ts @@ -0,0 +1,42 @@ +/** + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +/** + * @fileoverview Regex and parser for `[N]` and `[N, M]` citation markers. + * + * Single-source figures carry `[N]` and figures derived from multiple tables + * carry `[N, M]`, where each number is a 1-based index into the turn's + * provenance list. Both `chip_citation` and `markdown_table` parse markers + * through this module so prose rendering and table column hoisting match. + */ + +/** + * Returns a fresh global pattern over citation markers, whose first capture + * group holds the comma-separated numbers inside the brackets. Each call + * returns a new instance because a global `RegExp` tracks mutable `lastIndex` + * state. + */ +export function createCitationMarkerPattern(): RegExp { + return /\[(\d{1,2}(?:\s*,\s*\d{1,2})*)\]/g; +} + +/** + * Parses the 1-based source numbers from a marker's capture group, without + * duplicates, in order of first occurrence, so `[1, 1]` yields `[1]`. + */ +export function parseCitationNumbers(group: string): number[] { + return [...new Set(group.split(',').map((part) => Number(part.trim())))]; +} diff --git a/narratives/ui/src/utils/datacommons_component.tsx b/narratives/ui/src/utils/datacommons_component.tsx index 23c146232..2ae3cb3fa 100644 --- a/narratives/ui/src/utils/datacommons_component.tsx +++ b/narratives/ui/src/utils/datacommons_component.tsx @@ -204,23 +204,27 @@ export function dcChartRenderState( /** * Calls `onEmpty` once if the DC component under `hostRef` settles on having - * no data to draw. + * no data to draw, and `onSettled`, when given, once when the poll stops for + * any reason: drawn, empty, or {@link SETTLE_BUDGET_MS} exhausted. * * Polls rather than observing: the state we need appears inside the * component's shadow root, which does not exist yet at mount (Lit attaches it * on first update) and which a MutationObserver on the host cannot see into. * The poll stops as soon as the component settles either way, and gives up - * without reporting after {@link SETTLE_BUDGET_MS}. + * without reporting emptiness after {@link SETTLE_BUDGET_MS}. */ export function useDcChartEmptyNotice( hostRef: RefObject, tag: DataCommonsComponentTagName, onEmpty: () => void, + onSettled?: () => void, ): void { - // Held in a ref so an inline arrow from the caller does not restart the - // poll on every parent re-render (chat re-renders on every SSE chunk). + // Held in refs so inline arrows from the caller do not restart the poll on + // every parent re-render (chat re-renders on every SSE chunk). const onEmptyRef = useRef(onEmpty); onEmptyRef.current = onEmpty; + const onSettledRef = useRef(onSettled); + onSettledRef.current = onSettled; useEffect(() => { const deadline = Date.now() + SETTLE_BUDGET_MS; @@ -231,6 +235,7 @@ export function useDcChartEmptyNotice( if (state === 'pending' && Date.now() < deadline) return; window.clearInterval(timer); if (state === 'empty') onEmptyRef.current(); + onSettledRef.current?.(); }, SETTLE_POLL_MS); return () => window.clearInterval(timer); }, [hostRef, tag]); diff --git a/narratives/ui/src/utils/inspect_turn.test.ts b/narratives/ui/src/utils/inspect_turn.test.ts index b82d82658..11dbebf72 100644 --- a/narratives/ui/src/utils/inspect_turn.test.ts +++ b/narratives/ui/src/utils/inspect_turn.test.ts @@ -439,20 +439,30 @@ describe("inspectTurn", () => { expect(out.sourceChosenExplicitly).toBe(false); }); - it("maps the chart config the agent asked to render", () => { - const out = inspectTurn([], { - shouldRender: true, - charts: [ - { - vizType: "bar", - title: "Population across states", - variableDcids: ["Count_Person"], - placeDcids: ["wikidataId/Q1498"], - }, - ], - }); + it("summarizes the charts the turn renders", () => { + // Test: The chart summaries of an inspection. + // Situation: The turn renders a bar chart over one place and a ranking + // chart over a cohort, which names no place DCIDs. + // Expectation: Each chart is summarized by title, variables, and places, + // with an empty place list for the cohort chart. + const out = inspectTurn([], [ + { + vizType: "bar", + title: "Population across states", + variableDcids: ["Count_Person"], + placeDcids: ["wikidataId/Q1498"], + }, + { + vizType: "ranking", + title: "Median income by state", + variableDcids: ["Median_Income_Household"], + parentPlace: "country/USA", + childPlaceType: "State", + }, + ]); expect(out.charts).toEqual([ { title: "Population across states", variables: ["Count_Person"], places: ["wikidataId/Q1498"] }, + { title: "Median income by state", variables: ["Median_Income_Household"], places: [] }, ]); }); }); diff --git a/narratives/ui/src/utils/inspect_turn.ts b/narratives/ui/src/utils/inspect_turn.ts index 84ab1f391..eb82f530c 100644 --- a/narratives/ui/src/utils/inspect_turn.ts +++ b/narratives/ui/src/utils/inspect_turn.ts @@ -3,7 +3,7 @@ * data sources the answer was actually built on. */ -import type { ChartConfig, ProvenanceItem, ToolCallEvent } from "../hooks/use_sse_chat"; +import type { ChartItem, ProvenanceItem, ToolCallEvent } from "@/src/hooks/use_sse_chat"; /** * Tools that fetch observations and pin a source: server-side fetch functions @@ -255,13 +255,13 @@ function readPlacesWithData(payload: Record | null): string[] { /** * Summarizes what a turn actually used. * - * Everything is derived from data already in the browser -- the tool calls the - * agent streamed -- so this adds no request and cannot disagree with what the - * answer was built from. + * Everything is derived from data already in the browser (the streamed tool + * calls), so this adds no request and cannot disagree with what the answer was + * built from. `charts` is the list the turn renders from its chart blocks. */ export function inspectTurn( toolCalls: ToolCallEvent[], - chartConfig?: ChartConfig, + charts?: ChartItem[], _provenance?: ProvenanceItem[], ): TurnInspection { const calls = toolCalls ?? []; @@ -332,7 +332,7 @@ export function inspectTurn( } } - const charts = (chartConfig?.charts ?? []).map((chart) => ({ + const chartSummaries = (charts ?? []).map((chart) => ({ title: chart.title ?? "(untitled)", variables: chart.variableDcids ?? [], places: chart.placeDcids ?? [], @@ -344,6 +344,6 @@ export function inspectTurn( // Used facets first: those are what the answer rests on. facets: [...facets.values()].sort((a, b) => Number(b.used) - Number(a.used)), sourceChosenExplicitly, - charts, + charts: chartSummaries, }; } diff --git a/narratives/ui/src/utils/markdown_table.test.ts b/narratives/ui/src/utils/markdown_table.test.ts index 7c5798efd..1e4508084 100644 --- a/narratives/ui/src/utils/markdown_table.test.ts +++ b/narratives/ui/src/utils/markdown_table.test.ts @@ -75,6 +75,14 @@ describe('citationsIn', () => { expect(citationsIn('a [2] b [1][3]')).toEqual([2, 1, 3]); expect(citationsIn('no markers')).toEqual([]); }); + + it('reads every number of a composite marker', () => { + // Test: Composite marker extraction. + // Situation: A derived figure cites two sources as `[1, 2]`, followed by + // a single marker. + // Expectation: Both numbers of the composite marker, then the single one. + expect(citationsIn('**64.25%** [1, 2] and [3]')).toEqual([1, 2, 3]); + }); }); describe('analyzeMarkdownTable', () => { @@ -151,6 +159,21 @@ describe('analyzeMarkdownTable', () => { expect(hoistedCitations.get(1)).toEqual([1]); }); + it('hoists a shared composite marker and keeps the column numeric', () => { + // Test: Composite per-series citations in a value column. + // Situation: Every row of a derived value column cites `[1, 2]`. + // Expectation: Both source numbers hoist onto the header in order and + // stripping the composite marker leaves the column numeric. + const analysis = analyzeMarkdownTable( + seriesTable([ + ['2023', '64.25% [1, 2]'], + ['2022', '63.80% [1, 2]'], + ]), + ); + expect(analysis.hoistedCitations.get(1)).toEqual([1, 2]); + expect(analysis.numericColumns.has(1)).toBe(true); + }); + it('hoists past a placeholder row', () => { // Test: A gap in a single-source series. // Situation: One year reports "N/A" and so carries no marker. diff --git a/narratives/ui/src/utils/markdown_table.ts b/narratives/ui/src/utils/markdown_table.ts index 23f11d49a..86b3b043e 100644 --- a/narratives/ui/src/utils/markdown_table.ts +++ b/narratives/ui/src/utils/markdown_table.ts @@ -19,6 +19,10 @@ * its per-column citations. */ +import { + createCitationMarkerPattern, + parseCitationNumbers, +} from "./citation_marker"; import { isNeutralText, isNumericText, isYearText } from "./format_number"; /** @@ -33,9 +37,6 @@ export interface MarkdownNode { children?: MarkdownNode[]; } -/** A `[n]` citation marker, matching the range chip_citation renders. */ -const CITATION_MARKER = /\[(\d{1,2})\]/g; - /** What the table renderer needs to know that a single cell cannot tell it. */ export interface TableAnalysis { /** Columns whose body cells are all values — these are set right. */ @@ -57,11 +58,11 @@ export function textOf(node: MarkdownNode | undefined): string { /** The citation numbers in `text`, in the order they appear. */ export function citationsIn(text: string): number[] { + const pattern = createCitationMarkerPattern(); const out: number[] = []; let match: RegExpExecArray | null; - CITATION_MARKER.lastIndex = 0; - while ((match = CITATION_MARKER.exec(text)) !== null) { - out.push(Number(match[1])); + while ((match = pattern.exec(text)) !== null) { + out.push(...parseCitationNumbers(match[1])); } return out; } @@ -117,7 +118,7 @@ export function analyzeMarkdownTable( // Citations first: the markers have to come off before the rest of the // cell can be judged a number. const values = cells.map((text) => - text.replace(CITATION_MARKER, "").trim(), + text.replace(createCitationMarkerPattern(), "").trim(), ); const citations = cells .filter((_, i) => !isNeutralText(values[i])) @@ -128,7 +129,7 @@ export function analyzeMarkdownTable( key !== "" && citations.every((list) => list.join(",") === key) ) { - hoistedCitations.set(column, citations[0]); + hoistedCitations.set(column, [...new Set(citations[0])]); } const numeric = values.filter((text) => isNumericText(text)); diff --git a/narratives/ui/src/utils/rehype_stream_words.test.ts b/narratives/ui/src/utils/rehype_stream_words.test.ts index cdee99b96..c32ae8144 100644 --- a/narratives/ui/src/utils/rehype_stream_words.test.ts +++ b/narratives/ui/src/utils/rehype_stream_words.test.ts @@ -76,6 +76,20 @@ describe('rehypeStreamWords', () => { expect(item.properties?.style).toBe('color:red;animation-delay:0ms'); }); + + it('offsets every delay by the start time', () => { + // Test: Reveal offset. + // Situation: A two-word paragraph revealed over 100ms, starting at 300ms. + // Expectation: The words are delayed 300ms and 350ms. + const paragraph = element('p', [text('one two')]); + + rehypeStreamWords(100, 300)(element('root', [paragraph])); + + const delays = (paragraph.children ?? []) + .filter((node) => node.tagName === 'span') + .map((node) => node.properties?.style); + expect(delays).toEqual(['animation-delay:300ms', 'animation-delay:350ms']); + }); }); describe('calculateStreamDurationMs', () => { diff --git a/narratives/ui/src/utils/rehype_stream_words.ts b/narratives/ui/src/utils/rehype_stream_words.ts index 98d4ab1e1..770e8c791 100644 --- a/narratives/ui/src/utils/rehype_stream_words.ts +++ b/narratives/ui/src/utils/rehype_stream_words.ts @@ -44,18 +44,23 @@ export interface StreamNode { children?: StreamNode[]; } +/** Returns the number of whitespace-separated words in `text`. */ +export function countWords(text: string): number { + return text.split(/\s+/).filter(Boolean).length; +} + /** Returns how long the reveal of `text` runs, in milliseconds. */ export function calculateStreamDurationMs(text: string): number { - const words = text.split(/\s+/).filter(Boolean).length; - return Math.min(words * MS_PER_WORD, MAX_STREAM_MS); + return Math.min(countWords(text) * MS_PER_WORD, MAX_STREAM_MS); } /** * Wraps every word in a span and gives each an `animation-delay` that grows - * in reading order, spread across `durationMs`. Whitespace stays as plain - * text so wrapping and copy-paste are unchanged. + * in reading order, spread across `durationMs` and offset by `startMs`, so a + * body that follows another in one reveal starts where the previous one ends. + * Whitespace stays as plain text so wrapping and copy-paste are unchanged. */ -export function rehypeStreamWords(durationMs: number) { +export function rehypeStreamWords(durationMs: number, startMs = 0) { return (tree: StreamNode): void => { // Each entry is a node and the index of the word it appears with. const timed: [StreamNode, number][] = []; @@ -89,7 +94,7 @@ export function rehypeStreamWords(durationMs: number) { walk(tree); const step = durationMs / Math.max(words, 1); for (const [node, index] of timed) { - const delay = `animation-delay:${Math.round(index * step)}ms`; + const delay = `animation-delay:${Math.round(startMs + index * step)}ms`; const style = node.properties?.style; node.properties = { ...node.properties,