mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: keep the answer's citations and end a turn's reasoning with the turn
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ed0b71d4ff
commit
ee449fb579
@@ -4,7 +4,8 @@
|
||||
import GfmMarkdown from './GfmMarkdown.svelte'
|
||||
import AgentTrace from './AgentTrace.svelte'
|
||||
import LabeledDivider from './LabeledDivider.svelte'
|
||||
import { buildAgentTrace } from './agentTrace'
|
||||
import WebSearchSourcesDisplay from './copilot/chat/WebSearchSourcesDisplay.svelte'
|
||||
import { buildAgentTrace, splitFinalAnswer } from './agentTrace'
|
||||
import { runPane } from './agentScroll'
|
||||
import { createBottomSticker } from './stickToBottom'
|
||||
import { formatTokenCount, summarizeAgentResult, type AgentResult } from './aiAgentResult'
|
||||
@@ -29,13 +30,11 @@
|
||||
let summary = $derived(summarizeAgentResult(result))
|
||||
let textOutput = $derived(typeof result.output === 'string' ? result.output : undefined)
|
||||
|
||||
// The run's last message is what produced `output`, so it is dropped from the
|
||||
// trace: the output block below is that same text, and printing it twice in
|
||||
// one scroll reads as the agent having answered itself.
|
||||
let trace = $derived.by(() => {
|
||||
const entries = buildAgentTrace(result.messages)
|
||||
return entries.at(-1)?.kind === 'assistant' ? entries.slice(0, -1) : entries
|
||||
})
|
||||
// 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)
|
||||
|
||||
let anchor: HTMLElement | undefined = $state()
|
||||
const sticker = createBottomSticker()
|
||||
@@ -69,6 +68,11 @@
|
||||
{:else}
|
||||
{@render structuredOutput(result.output)}
|
||||
{/if}
|
||||
{#if answer.sources}
|
||||
<div class="mt-2">
|
||||
<WebSearchSourcesDisplay sources={answer.sources} />
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
<!-- What the run cost, as a footnote to what it produced. -->
|
||||
|
||||
@@ -30,6 +30,11 @@
|
||||
const continues = progress.key === streamKey && raw.length >= progress.consumed
|
||||
const base = continues ? progress : emptyAgentStreamProgress()
|
||||
progress = { ...advanceAgentStream(raw, base), key: streamKey }
|
||||
if (!continues) {
|
||||
// Another run, so the reader's decision to stop following the previous
|
||||
// one does not carry over: this one starts at its latest output.
|
||||
following = true
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -66,7 +71,6 @@
|
||||
sticker.scrollToEnd(runPane(anchor))
|
||||
}
|
||||
})
|
||||
|
||||
</script>
|
||||
|
||||
<!-- The same order a finished run uses — what it did, then what it is saying — so
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildAgentTrace } from './agentTrace'
|
||||
import { buildAgentTrace, splitFinalAnswer } from './agentTrace'
|
||||
import { parseAgentErrorMessages } from './aiAgentResult'
|
||||
import type { AgentMessage } from './aiAgentResult'
|
||||
|
||||
@@ -121,6 +121,50 @@ describe('buildAgentTrace', () => {
|
||||
// than reusing the success envelope's writer. `agent_action` is `skip_serializing`
|
||||
// on `OpenAIMessage`, so if that path ever stops wrapping them the tags vanish and
|
||||
// this trace silently empties — which is the one run worth reading.
|
||||
describe('splitFinalAnswer', () => {
|
||||
it('moves the answering turn out of the trace, with its citations', () => {
|
||||
const entries = buildAgentTrace([
|
||||
{ role: 'tool', content: 'Used websearch tool', agent_action: { type: 'web_search' } },
|
||||
{
|
||||
role: 'assistant',
|
||||
content: 'Postgres 17 changed the default.',
|
||||
annotations: [{ url: 'https://postgresql.org/docs', title: 'Release notes' }],
|
||||
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' }]
|
||||
})
|
||||
})
|
||||
|
||||
// A run whose last turn returned a tool call and no text leaves its answer
|
||||
// mid-trace. Looking only at the final entry finds nothing to move and prints
|
||||
// that answer as a row and again under the output.
|
||||
it('finds the answering turn even when it is not the last one', () => {
|
||||
const entries = buildAgentTrace([
|
||||
{ role: 'assistant', content: 'Let me check.', agent_action: { type: 'message' } },
|
||||
{
|
||||
role: 'tool',
|
||||
tool_call_id: 'call_1',
|
||||
content: '{}',
|
||||
agent_action: {
|
||||
type: 'tool_call',
|
||||
job_id: '0199-job',
|
||||
module_id: 'b',
|
||||
function_name: 'query_metrics'
|
||||
}
|
||||
}
|
||||
])
|
||||
expect(splitFinalAnswer(entries, 'Let me check.').trace).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 })
|
||||
})
|
||||
})
|
||||
|
||||
describe('the max-iterations path', () => {
|
||||
it('traces the partial messages the error carries', () => {
|
||||
const partial = parseAgentErrorMessages({
|
||||
|
||||
@@ -110,3 +110,35 @@ export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] {
|
||||
return entries
|
||||
}
|
||||
|
||||
/**
|
||||
* Separates the turn that produced the output from the rest of the trace, so a
|
||||
* view can render the answer once, under its own heading, with the citations
|
||||
* that belong to it.
|
||||
*
|
||||
* The turn is found by content, and from the end rather than at it. Every turn
|
||||
* that produced text is an entry while only one of them became the output, so a
|
||||
* run whose last turn returned a tool call and no text leaves its answer sitting
|
||||
* mid-trace — and anything that only inspects the final entry renders that answer
|
||||
* twice. Searching from the end is what makes two turns of identical text resolve
|
||||
* to the later one.
|
||||
*/
|
||||
export function splitFinalAnswer(
|
||||
entries: AgentTraceEntry[],
|
||||
output: unknown
|
||||
): { trace: AgentTraceEntry[]; sources?: WebSearchSource[] } {
|
||||
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 }
|
||||
}
|
||||
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 { trace: entries }
|
||||
}
|
||||
|
||||
@@ -92,7 +92,10 @@ describe('agent stream', () => {
|
||||
// The stream only grows, so each poll must fold in the new lines and re-read
|
||||
// none of the old ones — the reason this is incremental at all.
|
||||
it('resumes where the previous poll stopped', () => {
|
||||
const firstPoll = advanceAgentStream(lines.slice(0, 3).join('\n') + '\n', emptyAgentStreamProgress())
|
||||
const firstPoll = advanceAgentStream(
|
||||
lines.slice(0, 3).join('\n') + '\n',
|
||||
emptyAgentStreamProgress()
|
||||
)
|
||||
const secondPoll = advanceAgentStream(events, firstPoll)
|
||||
expect(secondPoll.consumed).toBe(events.length)
|
||||
expect(secondPoll.stream.current).toBe('eu-central-1 is down')
|
||||
@@ -136,7 +139,7 @@ describe('formatTokenCount', () => {
|
||||
// tool at once. Appending across that boundary makes the live answer read
|
||||
// "I'll checkThe issue is..." and never converge on the finished output.
|
||||
describe('a stream that narrates before calling a tool', () => {
|
||||
it('keeps only the turn that produced the answer', () => {
|
||||
it('turns the narration into a row and starts the answer fresh', () => {
|
||||
const raw = [
|
||||
'{"type":"token_delta","content":"Let me check the metrics."}',
|
||||
'{"type":"tool_call","call_id":"c1","function_name":"query_metrics"}',
|
||||
@@ -150,7 +153,7 @@ describe('a stream that narrates before calling a tool', () => {
|
||||
expect(stream.entries[0]).toEqual({ kind: 'assistant', content: 'Let me check the metrics.' })
|
||||
})
|
||||
|
||||
it('drops the narration at the boundary even across polls', () => {
|
||||
it('closes the turn at the boundary even when polls split it', () => {
|
||||
const first = '{"type":"token_delta","content":"Let me check."}\n'
|
||||
const afterCall = first + '{"type":"tool_call","call_id":"c1","function_name":"q"}\n'
|
||||
const poll1 = advanceAgentStream(first, emptyAgentStreamProgress())
|
||||
@@ -158,10 +161,27 @@ describe('a stream that narrates before calling a tool', () => {
|
||||
const poll2 = advanceAgentStream(afterCall, poll1)
|
||||
expect(poll2.stream.current).toBe('')
|
||||
expect(poll2.stream.entries[0]).toEqual({ kind: 'assistant', content: 'Let me check.' })
|
||||
const poll3 = advanceAgentStream(afterCall + '{"type":"token_delta","content":"Done."}\n', poll2)
|
||||
const poll3 = advanceAgentStream(
|
||||
afterCall + '{"type":"token_delta","content":"Done."}\n',
|
||||
poll2
|
||||
)
|
||||
expect(poll3.stream.current).toBe('Done.')
|
||||
})
|
||||
|
||||
// Extended thinking emits reasoning with no narration before the tool call, so
|
||||
// the turn boundary is the only thing that can end it.
|
||||
it('ends a turn that thought without narrating', () => {
|
||||
const raw = [
|
||||
'{"type":"reasoning_token_delta","content":"The user wants the metrics."}',
|
||||
'{"type":"tool_call","call_id":"c1","function_name":"q"}',
|
||||
'{"type":"tool_result","call_id":"c1","function_name":"q","result":"{}","success":true}',
|
||||
'{"type":"reasoning_token_delta","content":"eu-central-1 looks down."}',
|
||||
''
|
||||
].join('\n')
|
||||
const { stream } = advanceAgentStream(raw, emptyAgentStreamProgress())
|
||||
expect(stream.reasoning).toBe('eu-central-1 looks down.')
|
||||
})
|
||||
|
||||
// Bedrock has its own streaming implementation rather than the shared SSE
|
||||
// parsers, and never announces `tool_call` — only the arguments, then the
|
||||
// worker's `tool_execution`. Keying the reset on `tool_call` alone leaves the
|
||||
@@ -194,8 +214,14 @@ describe('coercing messages at the boundary', () => {
|
||||
it.each([
|
||||
['tool_calls that are not a list', { role: 'assistant', tool_calls: {} }],
|
||||
['a null entry inside tool_calls', { role: 'assistant', tool_calls: [null] }],
|
||||
['a tool call whose function is a string', { role: 'assistant', tool_calls: [{ id: 'c1', function: 'q' }] }],
|
||||
['non-string arguments', { role: 'assistant', tool_calls: [{ id: 'c1', function: { arguments: 3 } }] }],
|
||||
[
|
||||
'a tool call whose function is a string',
|
||||
{ role: 'assistant', tool_calls: [{ id: 'c1', function: 'q' }] }
|
||||
],
|
||||
[
|
||||
'non-string arguments',
|
||||
{ role: 'assistant', tool_calls: [{ id: 'c1', function: { arguments: 3 } }] }
|
||||
],
|
||||
['annotations that are not a list', { role: 'assistant', annotations: 'abc' }],
|
||||
['a null entry inside annotations', { role: 'assistant', annotations: [null] }],
|
||||
['an annotation with no url', { role: 'assistant', annotations: [{ title: 'x' }] }],
|
||||
@@ -209,7 +235,10 @@ describe('coercing messages at the boundary', () => {
|
||||
|
||||
it('keeps a well-formed call intact', () => {
|
||||
const parsed = messagesOf([
|
||||
{ role: 'assistant', tool_calls: [{ id: 'c1', type: 'function', function: { name: 'q', arguments: '{}' } }] }
|
||||
{
|
||||
role: 'assistant',
|
||||
tool_calls: [{ id: 'c1', type: 'function', function: { name: 'q', arguments: '{}' } }]
|
||||
}
|
||||
])
|
||||
expect(parsed?.[0].tool_calls).toEqual([
|
||||
{ id: 'c1', type: 'function', function: { name: 'q', arguments: '{}' } }
|
||||
|
||||
@@ -67,7 +67,9 @@ function toAgentMessage(raw: Record<string, unknown>): AgentMessage {
|
||||
}))
|
||||
: undefined
|
||||
const annotations = Array.isArray(raw.annotations)
|
||||
? raw.annotations.filter((a): a is Record<string, unknown> => isRecord(a) && typeof a.url === 'string')
|
||||
? raw.annotations.filter(
|
||||
(a): a is Record<string, unknown> => isRecord(a) && typeof a.url === 'string'
|
||||
)
|
||||
: undefined
|
||||
return {
|
||||
role: raw.role as string,
|
||||
@@ -299,13 +301,19 @@ export function advanceAgentStream(
|
||||
} else if (event.type === 'reasoning_token_delta' && typeof event.content === 'string') {
|
||||
stream.reasoning += event.content
|
||||
} else if (typeof event.function_name === 'string') {
|
||||
if (TOOL_TURN_STARTED.includes(event.type) && stream.current !== '') {
|
||||
// A model can narrate and request a tool in the same turn. The call
|
||||
// settles what that text was: narration, not the output. It becomes a
|
||||
// row rather than being dropped, so nothing vanishes from the screen
|
||||
// only to reappear when the result lands.
|
||||
stream.entries.push({ kind: 'assistant', content: stream.current })
|
||||
stream.current = ''
|
||||
if (TOOL_TURN_STARTED.includes(event.type)) {
|
||||
if (stream.current !== '') {
|
||||
// A model can narrate and request a tool in the same turn. The call
|
||||
// settles what that text was: narration, not the output. It becomes a
|
||||
// row rather than being dropped, so nothing vanishes from the screen
|
||||
// only to reappear when the result lands.
|
||||
stream.entries.push({ kind: 'assistant', content: stream.current })
|
||||
stream.current = ''
|
||||
}
|
||||
// Thinking belongs to the turn that produced it, and a turn can think
|
||||
// without narrating — extended thinking before a tool call is exactly
|
||||
// that shape. So this clears on the boundary itself, not with the
|
||||
// narration, or one turn's thoughts run into the next turn's.
|
||||
stream.reasoning = ''
|
||||
}
|
||||
// The same call is announced, then argued, then executed, then answered.
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
* Distance from the end within which a reader counts as still following. Allows
|
||||
* for sub-pixel rounding from `scrollTo` and the occasional overscroll bounce.
|
||||
*/
|
||||
export const STICK_TO_BOTTOM_PX = 8
|
||||
const STICK_TO_BOTTOM_PX = 8
|
||||
|
||||
/** A scroll event this close after our own scroll is ours, not the reader's. */
|
||||
const OWN_SCROLL_WINDOW_MS = 120
|
||||
|
||||
Reference in New Issue
Block a user