mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
refactor: coerce agent messages once at the parse boundary
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
c7c117a814
commit
c528ba41d4
@@ -144,17 +144,3 @@ describe('the max-iterations path', () => {
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
// A result is whatever a script returned, so every nested field is arbitrary even
|
||||
// once the messages have roles. A throw in here takes down the whole result
|
||||
// viewer, including the ordinary error such a payload usually rides on.
|
||||
describe('a malformed message that still has a role', () => {
|
||||
it.each([
|
||||
['tool_calls that are not a list', { role: 'assistant', tool_calls: {} }],
|
||||
['annotations that are not a list', { role: 'assistant', content: 'hi', annotations: 'abc' }],
|
||||
['a content object', { role: 'assistant', content: { text: 'hi' } }],
|
||||
['an agent_action that is not an object', { role: 'tool', agent_action: 'tool_call' }]
|
||||
])('survives %s', (_label, message) => {
|
||||
expect(() => buildAgentTrace([message as never])).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -46,13 +46,10 @@ function contentText(content: unknown): string {
|
||||
|
||||
function sourcesOf(message: AgentMessage): WebSearchSource[] | undefined {
|
||||
const annotations = message.annotations
|
||||
if (!Array.isArray(annotations) || annotations.length === 0) {
|
||||
if (!annotations?.length) {
|
||||
return undefined
|
||||
}
|
||||
const sources = annotations
|
||||
.filter((a) => typeof a?.url === 'string')
|
||||
.map((a) => ({ url: a.url, title: a.title }))
|
||||
return sources.length > 0 ? sources : undefined
|
||||
return annotations.map((a) => ({ url: a.url, title: a.title }))
|
||||
}
|
||||
|
||||
export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] {
|
||||
@@ -61,14 +58,7 @@ export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] {
|
||||
// the two are joined by `tool_call_id`.
|
||||
const argsByCallId = new Map<string, string>()
|
||||
for (const message of messages) {
|
||||
// A job result is whatever its script returned, and the shape check that got
|
||||
// us here only proves each message has a `role`. Anything nested is still
|
||||
// arbitrary, and a throw here would take the whole result viewer down with
|
||||
// it — including the plain error a lookalike payload is usually attached to.
|
||||
if (!Array.isArray(message.tool_calls)) {
|
||||
continue
|
||||
}
|
||||
for (const call of message.tool_calls) {
|
||||
for (const call of message.tool_calls ?? []) {
|
||||
if (call.id && typeof call.function?.arguments === 'string') {
|
||||
argsByCallId.set(call.id, call.function.arguments)
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildAgentTrace } from './agentTrace'
|
||||
import {
|
||||
advanceAgentStream,
|
||||
emptyAgentStreamProgress,
|
||||
@@ -170,3 +171,38 @@ describe('a stream that narrates before calling a tool', () => {
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
// A result is whatever a script returned, so a message that has a `role` still has
|
||||
// arbitrary anything underneath. Coercing once here is what lets every reader
|
||||
// treat `AgentMessage` as true; a bad value reaching them throws mid-render and
|
||||
// takes the result viewer down, including the plain error it usually rides on.
|
||||
describe('coercing messages at the boundary', () => {
|
||||
function messagesOf(raw: unknown) {
|
||||
return parseAgentResult({ output: '', messages: raw })?.messages
|
||||
}
|
||||
|
||||
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 } }] }],
|
||||
['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' }] }],
|
||||
['an agent_action that is not an object', { role: 'tool', agent_action: 'tool_call' }],
|
||||
['an agent_action with no type', { role: 'tool', agent_action: {} }]
|
||||
])('survives %s', (_label, message) => {
|
||||
const parsed = messagesOf([message])
|
||||
expect(parsed).toHaveLength(1)
|
||||
expect(() => buildAgentTrace(parsed!)).not.toThrow()
|
||||
})
|
||||
|
||||
it('keeps a well-formed call intact', () => {
|
||||
const parsed = messagesOf([
|
||||
{ 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: '{}' } }
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -43,6 +43,51 @@ function hasRole(message: unknown): boolean {
|
||||
return isRecord(message) && typeof message.role === 'string'
|
||||
}
|
||||
|
||||
/**
|
||||
* A job result is whatever its script returned, so a message that passed the
|
||||
* shape check still has arbitrary anything underneath. Everything downstream
|
||||
* reads these as `AgentMessage`, and a value of the wrong type there throws in
|
||||
* the middle of rendering, taking the whole result viewer with it.
|
||||
*
|
||||
* So the coercion happens once, here: past this point the declared type is the
|
||||
* real one, and no reader needs a guard of its own.
|
||||
*/
|
||||
function toAgentMessage(raw: Record<string, unknown>): AgentMessage {
|
||||
const toolCalls = Array.isArray(raw.tool_calls)
|
||||
? raw.tool_calls.filter(isRecord).map((call) => ({
|
||||
id: typeof call.id === 'string' ? call.id : undefined,
|
||||
type: typeof call.type === 'string' ? call.type : undefined,
|
||||
function: isRecord(call.function)
|
||||
? {
|
||||
name: typeof call.function.name === 'string' ? call.function.name : undefined,
|
||||
arguments:
|
||||
typeof call.function.arguments === 'string' ? call.function.arguments : undefined
|
||||
}
|
||||
: undefined
|
||||
}))
|
||||
: undefined
|
||||
const annotations = Array.isArray(raw.annotations)
|
||||
? raw.annotations.filter((a): a is Record<string, unknown> => isRecord(a) && typeof a.url === 'string')
|
||||
: undefined
|
||||
return {
|
||||
role: raw.role as string,
|
||||
content: raw.content,
|
||||
tool_calls: toolCalls,
|
||||
tool_call_id: typeof raw.tool_call_id === 'string' ? raw.tool_call_id : undefined,
|
||||
// The union is discriminated on `type`; an action without a string one
|
||||
// matches no branch and is treated as untagged.
|
||||
agent_action:
|
||||
isRecord(raw.agent_action) && typeof raw.agent_action.type === 'string'
|
||||
? (raw.agent_action as unknown as AgentAction)
|
||||
: undefined,
|
||||
annotations: annotations as AgentMessage['annotations']
|
||||
}
|
||||
}
|
||||
|
||||
function toAgentMessages(raw: unknown[]): AgentMessage[] {
|
||||
return raw.map((message) => toAgentMessage(message as Record<string, unknown>))
|
||||
}
|
||||
|
||||
/**
|
||||
* Recognise the envelope by its shape rather than by a marker key the worker
|
||||
* would have to add: sniffing works on runs that already completed, and the
|
||||
@@ -79,7 +124,7 @@ export function parseAgentResult(result: unknown): AgentResult | undefined {
|
||||
}
|
||||
return {
|
||||
output: result.output,
|
||||
messages: result.messages as AgentMessage[],
|
||||
messages: toAgentMessages(result.messages),
|
||||
usage: isRecord(result.usage) ? (result.usage as AgentTokenUsage) : undefined,
|
||||
wm_stream: typeof result.wm_stream === 'string' ? result.wm_stream : undefined
|
||||
}
|
||||
@@ -101,7 +146,7 @@ export function parseAgentErrorMessages(result: unknown): AgentMessage[] | undef
|
||||
if (inner.messages.length === 0 || !inner.messages.every(hasRole)) {
|
||||
return undefined
|
||||
}
|
||||
return inner.messages as AgentMessage[]
|
||||
return toAgentMessages(inner.messages)
|
||||
}
|
||||
|
||||
export type AgentResultSummary = {
|
||||
|
||||
Reference in New Issue
Block a user