diff --git a/frontend/src/lib/components/AgentResultDisplay.svelte b/frontend/src/lib/components/AgentResultDisplay.svelte index 0388ab0845..ad84cc263b 100644 --- a/frontend/src/lib/components/AgentResultDisplay.svelte +++ b/frontend/src/lib/components/AgentResultDisplay.svelte @@ -5,7 +5,6 @@ import AgentTrace from './AgentTrace.svelte' import ChatCollapsibleCard from './copilot/chat/ChatCollapsibleCard.svelte' import LabeledDivider from './LabeledDivider.svelte' - import WebSearchSourcesDisplay from './copilot/chat/WebSearchSourcesDisplay.svelte' import { buildAgentTrace, splitFinalAnswer } from './agentTrace' import { runPane } from './agentScroll' import { createBottomSticker } from './stickToBottom' @@ -32,9 +31,8 @@ // The turn that produced `output` is not a trace row: the output block below is // that same text, and printing it twice in one scroll reads as the agent having - // answered itself. Its citations move down with it. - let answer = $derived(splitFinalAnswer(buildAgentTrace(result.messages), result.output)) - let trace = $derived(answer.trace) + // answered itself. Its citations stay in the trace, on the search that found them. + let trace = $derived(splitFinalAnswer(buildAgentTrace(result.messages), result.output)) let reasoning = $derived(result.reasoning?.trim()) let reasoningExpanded = $state(false) @@ -85,11 +83,6 @@ {:else} {@render structuredOutput(result.output)} {/if} - {#if answer.sources} -
- -
- {/if} diff --git a/frontend/src/lib/components/AgentTrace.svelte b/frontend/src/lib/components/AgentTrace.svelte index 91b87fea67..7f2a90e621 100644 --- a/frontend/src/lib/components/AgentTrace.svelte +++ b/frontend/src/lib/components/AgentTrace.svelte @@ -6,6 +6,7 @@ import ChatCollapsibleCard from './copilot/chat/ChatCollapsibleCard.svelte' import ToolContentDisplay from './copilot/chat/ToolContentDisplay.svelte' import WebSearchSourcesDisplay from './copilot/chat/WebSearchSourcesDisplay.svelte' + import { webSearchResultOf, type WebSearchResult } from './copilot/chat/shared' import GfmMarkdown from './GfmMarkdown.svelte' import type { AgentTraceEntry } from './agentTrace' import { SvelteMap, SvelteSet } from 'svelte/reactivity' @@ -21,11 +22,14 @@ let { entries, workspaceId }: Props = $props() let expanded = new SvelteSet() + // A search opens on its sources, so for it the reader's toggle records a close. + let collapsedSearches = new SvelteSet() // Row state is keyed by position, which means nothing once the viewer is handed // another run: row 0 would stay open showing the previous run's job. $effect(() => { entries expanded.clear() + collapsedSearches.clear() jobs.clear() }) // A tool's own job holds what the envelope does not: its logs, how long it @@ -67,12 +71,18 @@ return typeof job === 'object' ? job : undefined } - function toolLabel(entry: Extract): string { - const job = jobOf(entry.jobId) - const duration = job?.['duration_ms'] - return duration === undefined - ? entry.name - : `${entry.name} · ${msToReadableTimeShort(duration)}` + function toolLabel( + entry: Extract, + search: WebSearchResult | undefined + ): string { + const duration = jobOf(entry.jobId)?.['duration_ms'] + return [ + entry.name, + search?.query !== undefined ? `"${search.query}"` : undefined, + duration !== undefined ? msToReadableTimeShort(duration) : undefined + ] + .filter((part) => part !== undefined) + .join(' · ') } @@ -81,25 +91,29 @@ {#if entry.kind === 'assistant'}
- {#if entry.sources} -
- -
- {/if}
{:else if entry.kind === 'search'} - + {}} - /> + label="Searched the web" + expanded={Boolean(entry.sources) && !collapsedSearches.has(index)} + toggleable={Boolean(entry.sources)} + onToggle={() => { + if (!collapsedSearches.delete(index)) collapsedSearches.add(index) + }} + > + {#if entry.sources} + + {/if} + {:else} {@const job = jobOf(entry.jobId)} + {@const failed = job?.type === 'CompletedJob' && !job.success} + {@const search = webSearchResultOf(entry.result)} toggle(index, entry)} contentClass="space-y-3" @@ -110,11 +124,15 @@ {#if job?.logs} {/if} - + {#if search && !failed} + + {:else} + + {/if} {#if entry.resourcePath}
diff --git a/frontend/src/lib/components/agentTrace.test.ts b/frontend/src/lib/components/agentTrace.test.ts index 3297a99d82..7d1dcf3429 100644 --- a/frontend/src/lib/components/agentTrace.test.ts +++ b/frontend/src/lib/components/agentTrace.test.ts @@ -45,7 +45,7 @@ describe('buildAgentTrace', () => { result: '{"eu-central-1":0.184}', jobId: '0199-job' }, - { kind: 'assistant', content: 'eu-central-1 is down.', sources: undefined } + { kind: 'assistant', content: 'eu-central-1 is down.' } ]) }) @@ -76,8 +76,8 @@ describe('buildAgentTrace', () => { // The worker splits a search the same way: a `tool` message tagged web_search // carrying a constant sentence, then the assistant turn that carries the - // citations. The search row therefore has nothing of its own to show. - it('records the search and puts its citations on the turn that follows', () => { + // citations, which move onto the search so it shows what it found. + it('puts the citations of the turn that follows on the search', () => { const entries = buildAgentTrace([ { role: 'tool', @@ -92,12 +92,11 @@ describe('buildAgentTrace', () => { } ]) expect(entries).toEqual([ - { kind: 'search' }, { - kind: 'assistant', - content: 'Postgres 17 changed the default.', + kind: 'search', sources: [{ url: 'https://postgresql.org/docs', title: 'Release notes' }] - } + }, + { kind: 'assistant', content: 'Postgres 17 changed the default.' } ]) }) @@ -118,7 +117,7 @@ describe('buildAgentTrace', () => { }) describe('splitFinalAnswer', () => { - it('moves the answering turn out of the trace, with its citations', () => { + it('moves the answering turn out of the trace, leaving its citations on the search', () => { const entries = buildAgentTrace([ { role: 'tool', content: 'Used websearch tool', agent_action: { type: 'web_search' } }, { @@ -128,10 +127,12 @@ describe('splitFinalAnswer', () => { agent_action: { type: 'message' } } ]) - expect(splitFinalAnswer(entries, 'Postgres 17 changed the default.')).toEqual({ - trace: [{ kind: 'search' }], - sources: [{ url: 'https://postgresql.org/docs', title: 'Release notes' }] - }) + expect(splitFinalAnswer(entries, 'Postgres 17 changed the default.')).toEqual([ + { + kind: 'search', + sources: [{ url: 'https://postgresql.org/docs', title: 'Release notes' }] + } + ]) }) // A run whose last turn returned a tool call and no text leaves its answer @@ -152,12 +153,12 @@ describe('splitFinalAnswer', () => { } } ]) - expect(splitFinalAnswer(entries, 'Let me check.').trace).toEqual([entries[1]]) + expect(splitFinalAnswer(entries, 'Let me check.')).toEqual([entries[1]]) }) it('leaves the trace whole when the output is not a turn of its own', () => { const entries = buildAgentTrace(messages) - expect(splitFinalAnswer(entries, { rows: 3 })).toEqual({ trace: entries }) + expect(splitFinalAnswer(entries, { rows: 3 })).toEqual(entries) }) }) diff --git a/frontend/src/lib/components/agentTrace.ts b/frontend/src/lib/components/agentTrace.ts index ba0cb78d4e..392e8b00c9 100644 --- a/frontend/src/lib/components/agentTrace.ts +++ b/frontend/src/lib/components/agentTrace.ts @@ -8,10 +8,10 @@ import type { AgentMessage } from './aiAgentResult' * are shown as inputs, while the trace is what the agent did with them. */ export type AgentTraceEntry = - | { kind: 'assistant'; content: string; sources?: WebSearchSource[] } - /** A search records only that one happened: the worker writes a constant - * sentence, and the citations ride on the assistant turn that follows. */ - | { kind: 'search' } + | { kind: 'assistant'; content: string } + /** The worker records only that a search happened. Its citations ride on the assistant + * turn that follows, and are moved here so the search shows what it found. */ + | { kind: 'search'; sources?: WebSearchSource[] } | { kind: 'tool' name: string @@ -99,8 +99,14 @@ export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] { continue } const content = contentText(message.content) + // The worker writes a provider call's search right before that call's answer, and + // only a call that searched carries annotations. + const previous = entries[entries.length - 1] + if (message.role === 'assistant' && previous?.kind === 'search' && !previous.sources) { + previous.sources = sourcesOf(message) + } if (message.role === 'assistant' && content !== '') { - entries.push({ kind: 'assistant', content, sources: sourcesOf(message) }) + entries.push({ kind: 'assistant', content }) } } return entries @@ -108,27 +114,21 @@ export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] { /** * Separates the turn that produced the output from the rest of the trace, so the - * answer is rendered once with the citations that belong to it. Found by content - * and searched from the end: a run whose last turn returned a tool call leaves its - * answer mid-trace, where inspecting only the final entry prints it twice. + * answer is rendered once. Found by content and searched from the end: a run whose + * last turn returned a tool call leaves its answer mid-trace, where inspecting only + * the final entry prints it twice. */ -export function splitFinalAnswer( - entries: AgentTraceEntry[], - output: unknown -): { trace: AgentTraceEntry[]; sources?: WebSearchSource[] } { +export function splitFinalAnswer(entries: AgentTraceEntry[], output: unknown): AgentTraceEntry[] { if (typeof output !== 'string') { // A schema-shaped output is rendered by the result viewer itself and matches // no turn, so the whole trace stands. - return { trace: entries } + return entries } for (let i = entries.length - 1; i >= 0; i--) { const entry = entries[i] if (entry.kind === 'assistant' && entry.content === output) { - // The citations are an annotation on that turn, so moving the turn without - // them would leave a run that shows a web search ran and no source for - // what it answered. - return { trace: [...entries.slice(0, i), ...entries.slice(i + 1)], sources: entry.sources } + return [...entries.slice(0, i), ...entries.slice(i + 1)] } } - return { trace: entries } + return entries } diff --git a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte index 5405a6ea25..8d945f94d1 100644 --- a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte @@ -26,7 +26,7 @@ import { getChatViewHost } from './chatViewHost' const chatHost = getChatViewHost() - import { isActiveUserQuestion, type ToolDisplayMessage } from './shared' + import { isActiveUserQuestion, webSearchResultOf, type ToolDisplayMessage } from './shared' import ChatCollapsibleCard from './ChatCollapsibleCard.svelte' import { twMerge } from 'tailwind-merge' import { slide } from 'svelte/transition' @@ -140,6 +140,18 @@ message.previewCard && !message.isLoading && !message.error && !message.needsConfirmation ) ) + + // A provider-side search sets `webSearchSources` and words its own header; any other tool + // gets the same card by returning the web search result shape. + const searchResult = $derived( + message.webSearchSources || message.error ? undefined : webSearchResultOf(message.result) + ) + const sources = $derived(message.webSearchSources ?? searchResult?.sources) + const label = $derived( + searchResult?.query !== undefined + ? `${message.content} · "${searchResult.query}"` + : message.content + ) {#if activeUserQuestion} @@ -281,7 +293,7 @@ weight alone: queued calls (waiting their turn behind the executing tool) are faded, the running one sweeps, a settled one is plain. --> (isExpanded = !isExpanded)} toggleable={detailsAvailable || message.isStreamingArguments === true} @@ -343,8 +355,8 @@ {#if visibleActions.length > 0} - {:else if message.webSearchSources?.length && !message.error} - + {:else if sources?.length && !message.error} + {:else} import { Globe } from 'lucide-svelte' import { SvelteSet } from 'svelte/reactivity' - import type { WebSearchSource } from './shared' + import { isRenderableSourceUrl, type WebSearchSource } from './shared' import { isOfflineReplay } from '$lib/components/recording/offlineReplay.svelte' interface Props { sources: WebSearchSource[] + /** Whether a hostname may be sent to the favicon service. False for the sources a + * workspace's own tool returned: those can name internal services, and disclosing + * them to a third party is the tool author's call to make, not this card's. */ + favicons?: boolean } - let { sources }: Props = $props() + let { sources, favicons = true }: Props = $props() - // The URLs come from the provider's response: only render absolute http(s) - // ones — anything else (javascript:, data:, relative) must not become an - // href. Also dedupes; providers can surface the same page several times. + // The URLs come from the provider's response or from a tool's own result: only render + // absolute http(s) ones — anything else (javascript:, data:, relative) must not become + // an href. Also dedupes; providers can surface the same page several times. const uniqueSources = $derived( Array.from( - new Map( - sources - .filter((s) => { - try { - return ['http:', 'https:'].includes(new URL(s.url).protocol) - } catch { - return false - } - }) - .map((s) => [s.url, s]) - ).values() + new Map(sources.filter((s) => isRenderableSourceUrl(s.url)).map((s) => [s.url, s])).values() ) ) @@ -43,7 +37,7 @@ // to issue none — a recording comes from an arbitrary origin, so its cited // hostnames must not leak from a viewer's browser either. Degrades to the same // Globe the blocked/failed case already uses. - const noFavicons = $derived(isOfflineReplay()) + const noFavicons = $derived(!favicons || isOfflineReplay()) // Favicons come from Google's public favicon service, which discloses each // consulted hostname to a third party from the user's browser — an accepted @@ -62,6 +56,9 @@
{#each uniqueSources as source (source.url)} {@const hostname = hostnameOf(source.url)} + + {@const title = source.title?.trim() || undefined} failedFavicons.add(hostname)} /> {/if} - {source.title ?? hostname} - {#if source.title} + {title ?? hostname} + {#if title} {hostname} {/if} diff --git a/frontend/src/lib/components/copilot/chat/shared.test.ts b/frontend/src/lib/components/copilot/chat/shared.test.ts index 005a7666db..fb61f3a912 100644 --- a/frontend/src/lib/components/copilot/chat/shared.test.ts +++ b/frontend/src/lib/components/copilot/chat/shared.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from 'vitest' import { z } from 'zod' import type { DisplayMessage, ToolDisplayMessage } from './shared' -import { openItemPreviewAction } from './shared' +import { openItemPreviewAction, webSearchResultOf } from './shared' vi.mock('monaco-editor', () => ({ editor: {} @@ -1869,3 +1869,38 @@ describe('processToolCall confirmation hooks', () => { expect(tool.fn).toHaveBeenCalled() }) }) + +// Any tool gets the web search card by returning this shape. The card renders urls and +// titles only, so a result carrying anything more must keep its JSON pane. +describe('webSearchResultOf', () => { + it('reads the shape, from the value or its JSON text', () => { + const result = { sources: [{ url: 'https://a.dev', title: 'A' }], query: 'a' } + expect(webSearchResultOf(result)).toEqual(result) + expect(webSearchResultOf(JSON.stringify(result))).toEqual(result) + }) + + // A Python tool serializes an absent optional as null, and the card has nothing to tell + // its author why an almost-right result fell back to JSON. + it('reads null on an optional field as absent', () => { + expect( + webSearchResultOf({ sources: [{ url: 'https://a.dev', title: null }], query: null }) + ).toEqual({ + sources: [{ url: 'https://a.dev', title: undefined }], + query: undefined + }) + }) + + it.each([ + ['an extra key', { sources: [{ url: 'https://a.dev' }], summary: 'x' }], + ['an extra source field', { sources: [{ url: 'https://a.dev', snippet: 'x' }] }], + ['a source without url', { sources: [{ title: 'A' }] }], + // The card renders no relative or javascript: link, so a result whose urls it would + // drop keeps its own JSON rather than showing an empty source list. + ['a relative url', { sources: [{ url: '/docs/pg17', title: 'PG 17' }] }], + ['a javascript: url', { sources: [{ url: 'javascript:alert(1)' }] }], + ['no sources', { sources: [] }], + ['a bare list of links', [{ url: 'https://a.dev' }]] + ])('rejects %s', (_, result) => { + expect(webSearchResultOf(result)).toBeUndefined() + }) +}) diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index f58ed3b6d3..772284865d 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -610,6 +610,77 @@ export type WebSearchSource = { title?: string } +/** The result shape any tool returns to have it rendered as a web search card. */ +export type WebSearchResult = { + sources: WebSearchSource[] + query?: string +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +/** A link the card can render: the source list drops everything else, so a result whose + * urls are relative or `javascript:` would show as an empty list in place of its own output. */ +export function isRenderableSourceUrl(url: string): boolean { + try { + return ['http:', 'https:'].includes(new URL(url).protocol) + } catch { + return false + } +} + +/** An absent optional field reaches JSON as `null` from a Python tool and as nothing from a + * TypeScript one, so both read as absent. */ +function isOptionalString(value: unknown): boolean { + return value === null || typeof value === 'string' +} + +function isWebSearchSource(value: unknown): value is { url: string; title?: unknown } { + return ( + isRecord(value) && + typeof value.url === 'string' && + isRenderableSourceUrl(value.url) && + Object.entries(value).every( + ([key, v]) => key === 'url' || (key === 'title' && isOptionalString(v)) + ) + ) +} + +/** + * A tool result as a web search, or undefined when it is anything else. The keys must be + * exactly `sources` and an optional `query`: the card renders only urls and titles, so a + * result carrying more would lose it. Accepts the result as its JSON text too. + */ +export function webSearchResultOf(result: unknown): WebSearchResult | undefined { + if (typeof result === 'string') { + try { + result = JSON.parse(result) + } catch { + return undefined + } + } + if (!isRecord(result) || !Array.isArray(result.sources) || result.sources.length === 0) { + return undefined + } + const sources = result.sources + if ( + !sources.every(isWebSearchSource) || + !Object.entries(result).every( + ([key, v]) => key === 'sources' || (key === 'query' && isOptionalString(v)) + ) + ) { + return undefined + } + return { + sources: sources.map((source) => ({ + url: source.url, + title: typeof source.title === 'string' ? source.title : undefined + })), + query: typeof result.query === 'string' ? result.query : undefined + } +} + export type ToolDisplayMessage = { role: 'tool' tool_call_id: string diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts index 6ef18b7212..077090aba2 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts @@ -3,7 +3,7 @@ import type { ChatSendRequestOptions, ChatViewHost } from '$lib/components/copilot/chat/chatViewHost' -import type { DisplayMessage } from '$lib/components/copilot/chat/shared' +import type { DisplayMessage, WebSearchResult } from '$lib/components/copilot/chat/shared' import type { AIAutonomyMode } from '$lib/components/copilot/chat/AIChatManager.svelte' import { isPlanCardTool } from '$lib/components/copilot/chat/planMode' import { AttachedFilesStore } from '$lib/components/copilot/chat/files/attachedFiles.svelte' @@ -125,6 +125,33 @@ function toolErrorText(result: unknown): string | undefined { return undefined } +/** + * The provider-side search's row as the web search result shape, or undefined for any other + * row. The worker words it like a tool named `websearch`, which an agent tool may also be + * called; what tells them apart is that every agent tool's row stores its arguments, and the + * search stores none, only the citations it produced. + */ +function builtInWebSearch(message: ChatMessage): WebSearchResult | undefined { + const tool = message.tool + if (tool?.name !== 'websearch' || tool.arguments !== undefined || message.pending) { + return undefined + } + const citations = parseToolPayload(tool.result) + if (citations === undefined) return { sources: [] } + if ( + !Array.isArray(citations) || + !citations.every((c) => c && typeof c === 'object' && typeof c.url === 'string') + ) { + return undefined + } + return { + sources: citations.map((c: { url: string; title?: unknown }) => ({ + url: c.url, + title: typeof c.title === 'string' ? c.title : undefined + })) + } +} + /** The name the worker gives the structured-output tool: suffixed when an agent tool already has it. */ const STRUCTURED_OUTPUT_CALL = /^structured_output(_\d+)?$/ @@ -168,6 +195,25 @@ export function toDisplayMessages( ] } case 'tool': { + const search = builtInWebSearch(message) + if (search) { + const found = search.sources.length > 0 + // `autoCollapseDetails: false` opens it on its sources, as the session chat + // shows a search. A tool that returns the same shape stays collapsed like + // every other tool row, however many times the agent called it. + return [ + { + role: 'tool', + tool_call_id: message.id, + content: 'Searched the web', + toolName: 'web_search', + webSearchSources: found ? search.sources : undefined, + showDetails: found, + autoCollapseDetails: false, + jobId: message.jobId + } + ] + } // The model's call and what the tool sent back, as the stream carried them or the // worker stored them on the row. A row stored without them shows its name and job. const toolName = message.tool?.name diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts index 53fd7b6ab9..a0ae4b8092 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts @@ -125,6 +125,42 @@ describe('toDisplayMessages', () => { }) }) + // The worker words the provider-side search like a tool named `websearch`, which an agent + // tool may also be called. The search stores no arguments, and an agent tool always does. + it('renders the provider-side search as a web search card, not an agent tool of that name', () => { + const citations = JSON.stringify([ + { start_index: 0, end_index: 4, url: 'https://postgresql.org/docs', title: 'Release notes' } + ]) + const [search, agentTool, notCitations] = toDisplayMessages([ + message({ + role: 'tool', + content: 'Used websearch tool', + tool: { name: 'websearch', status: 'success', result: citations } + }), + message({ + role: 'tool', + content: 'Used websearch tool', + tool: { name: 'websearch', status: 'success', arguments: '{}', result: citations } + }), + // The safety valve of a name-based rule: a row that is not the provider's search + // keeps its result, whatever the tool is called. + message({ + role: 'tool', + content: 'Used websearch tool', + tool: { name: 'websearch', status: 'success', result: '{"hits":3}' } + }) + ]) + expect(search).toMatchObject({ + content: 'Searched the web', + webSearchSources: [{ url: 'https://postgresql.org/docs', title: 'Release notes' }], + showDetails: true, + autoCollapseDetails: false + }) + expect(agentTool).toMatchObject({ content: 'Used websearch tool', toolName: 'websearch' }) + expect(agentTool).not.toHaveProperty('webSearchSources') + expect(notCitations).toMatchObject({ toolName: 'websearch', result: { hits: 3 } }) + }) + it('shows a call the turn finished without as an error, not as still running', () => { const display = toDisplayMessages([ message({ role: 'user', content: 'hi' }),