mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(ai-agent): keep offline replay inert and the streamed answer to one turn
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
25d429a41a
commit
b5389bc075
@@ -79,6 +79,18 @@ lazy_static::lazy_static! {
|
||||
const DEFAULT_MAX_AGENT_ITERATIONS: usize = 10;
|
||||
const HARD_MAX_AGENT_ITERATIONS: usize = 1000;
|
||||
|
||||
/// What a run stopped by `max_iterations` reports back: the conversation it got
|
||||
/// through before giving up.
|
||||
///
|
||||
/// `Message` rather than `OpenAIMessage` is load-bearing. `agent_action` is
|
||||
/// `skip_serializing` on `OpenAIMessage` and only reaches JSON through this
|
||||
/// wrapper, so serializing these raw drops every tool name and job id and leaves
|
||||
/// the partial run unreadable — which is the one thing worth having on this path.
|
||||
#[derive(serde::Serialize)]
|
||||
struct MaxIterPartialResult<'a> {
|
||||
messages: Vec<Message<'a>>,
|
||||
}
|
||||
|
||||
fn strip_system_messages(messages: &[OpenAIMessage]) -> Vec<OpenAIMessage> {
|
||||
messages
|
||||
.iter()
|
||||
@@ -1614,14 +1626,6 @@ pub async fn run_agent(
|
||||
step_id: Option<&'a str>,
|
||||
result: MaxIterPartialResult<'a>,
|
||||
}
|
||||
// Through the same wrapper the success envelope uses: `agent_action`
|
||||
// is `skip_serializing` on `OpenAIMessage`, so serializing these raw
|
||||
// would drop every tool name and job id and leave the partial run
|
||||
// unreadable — which is the one thing worth having on this path.
|
||||
#[derive(serde::Serialize)]
|
||||
struct MaxIterPartialResult<'a> {
|
||||
messages: Vec<Message<'a>>,
|
||||
}
|
||||
return Err(Error::ExecutionRawError(
|
||||
serde_json::value::to_raw_value(&MaxIterError {
|
||||
message: format!(
|
||||
@@ -1899,6 +1903,37 @@ mod tests {
|
||||
assert!(!streaming_requested(Some(false)));
|
||||
}
|
||||
|
||||
/// The frontend reads a capped run's partial actions out of this payload and
|
||||
/// keys entirely off `agent_action`. That field is `skip_serializing` on
|
||||
/// `OpenAIMessage`, so serializing these messages directly rather than through
|
||||
/// `Message` silently empties the trace of the one run worth reading.
|
||||
#[test]
|
||||
fn max_iterations_partial_result_keeps_the_action_tags() {
|
||||
let messages = vec![OpenAIMessage {
|
||||
role: "tool".to_string(),
|
||||
content: Some(OpenAIContent::Text("{\"rows\":2}".to_string())),
|
||||
tool_call_id: Some("call_1".to_string()),
|
||||
agent_action: Some(AgentAction::ToolCall {
|
||||
job_id: uuid::Uuid::nil(),
|
||||
function_name: "list_payouts".to_string(),
|
||||
module_id: "b".to_string(),
|
||||
}),
|
||||
..Default::default()
|
||||
}];
|
||||
|
||||
let partial = MaxIterPartialResult {
|
||||
messages: messages
|
||||
.iter()
|
||||
.map(|m| Message { message: m, agent_action: m.agent_action.as_ref() })
|
||||
.collect(),
|
||||
};
|
||||
let json = serde_json::to_value(&partial).unwrap();
|
||||
|
||||
let action = &json["messages"][0]["agent_action"];
|
||||
assert_eq!(action["type"], "tool_call");
|
||||
assert_eq!(action["function_name"], "list_payouts");
|
||||
}
|
||||
|
||||
/// Over 64 characters OpenAI rejects the key outright, which costs a wasted round
|
||||
/// trip per run and silently leaves that step with no prompt caching at all.
|
||||
#[test]
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
import { Button } from '$lib/components/common'
|
||||
import GfmMarkdown from './GfmMarkdown.svelte'
|
||||
import Required from './Required.svelte'
|
||||
import autosize from '$lib/autosize'
|
||||
import TextInput from './text_input/TextInput.svelte'
|
||||
|
||||
interface Props {
|
||||
description: string
|
||||
@@ -35,6 +35,9 @@
|
||||
unifiedSize="xs"
|
||||
btnClasses={editing ? 'bg-surface-hover' : ''}
|
||||
startIcon={{ icon: Pen }}
|
||||
iconOnly
|
||||
title={editing ? 'Stop editing the description' : 'Edit the description'}
|
||||
aria-label={editing ? 'Stop editing the description' : 'Edit the description'}
|
||||
on:click={() => (editing = !editing)}
|
||||
/>
|
||||
{/if}
|
||||
@@ -42,13 +45,11 @@
|
||||
{#if canWrite && editing}
|
||||
<div class="relative">
|
||||
<div class="text-2xs text-primary absolute -top-4 right-0">GH Markdown</div>
|
||||
<textarea
|
||||
class="text-xs text-primary font-normal"
|
||||
disabled={!canWrite}
|
||||
use:autosize
|
||||
<TextInput
|
||||
underlyingInputEl="textarea"
|
||||
bind:value={description}
|
||||
{placeholder}
|
||||
></textarea>
|
||||
inputProps={{ placeholder, 'aria-label': label, disabled: !canWrite }}
|
||||
/>
|
||||
</div>
|
||||
{:else if description == undefined || description == ''}
|
||||
<div class="text-xs text-secondary font-normal">No description provided</div>
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
<!-- Shown above the Path input wherever a resource is created: the resource form and the
|
||||
connect drawer's last step. One component so the two screens cannot drift apart. -->
|
||||
<!-- Shown above the Path input wherever a resource is created: the resource form, the
|
||||
connect drawer's last step, and the save-as-reusable-agent drawer. One component so
|
||||
those screens cannot drift apart. -->
|
||||
<div class="text-xs text-secondary font-normal mb-1">
|
||||
The path sets who can access this resource: a <code>u/</code> path is private to that user, an
|
||||
<code>f/</code> path follows the folder's permissions — read access lets people use the resource, write
|
||||
|
||||
@@ -123,3 +123,32 @@ describe('formatTokenCount', () => {
|
||||
expect(formatTokenCount(48211)).toBe('48k')
|
||||
})
|
||||
})
|
||||
|
||||
// The worker streams every iteration's text, and a turn may narrate and call a
|
||||
// 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', () => {
|
||||
const raw = [
|
||||
'{"type":"token_delta","content":"Let me check the metrics."}',
|
||||
'{"type":"tool_call","call_id":"c1","function_name":"query_metrics"}',
|
||||
'{"type":"tool_result","call_id":"c1","function_name":"query_metrics","result":"{}","success":true}',
|
||||
'{"type":"token_delta","content":"eu-central-1 is down."}',
|
||||
''
|
||||
].join('\n')
|
||||
expect(advanceAgentStream(raw, emptyAgentStreamProgress()).stream.answer).toBe(
|
||||
'eu-central-1 is down.'
|
||||
)
|
||||
})
|
||||
|
||||
it('drops the narration at the boundary even across polls', () => {
|
||||
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())
|
||||
expect(poll1.stream.answer).toBe('Let me check.')
|
||||
const poll2 = advanceAgentStream(afterCall, poll1)
|
||||
expect(poll2.stream.answer).toBe('')
|
||||
const poll3 = advanceAgentStream(afterCall + '{"type":"token_delta","content":"Done."}\n', poll2)
|
||||
expect(poll3.stream.answer).toBe('Done.')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -232,6 +232,14 @@ 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 (event.type === 'tool_call') {
|
||||
// A model can narrate and request a tool in the same turn, and the loop
|
||||
// then runs again. That narration is not part of the answer — the
|
||||
// finished result keeps only the last turn's text — so a new call
|
||||
// starts the answer over rather than appending to what came before.
|
||||
stream.answer = ''
|
||||
stream.reasoning = ''
|
||||
}
|
||||
stream.tool = {
|
||||
name: event.function_name,
|
||||
running: event.type !== 'tool_result',
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import { Globe } from 'lucide-svelte'
|
||||
import { SvelteSet } from 'svelte/reactivity'
|
||||
import type { WebSearchSource } from './shared'
|
||||
import { isOfflineReplay } from '$lib/components/recording/offlineReplay.svelte'
|
||||
|
||||
interface Props {
|
||||
sources: WebSearchSource[]
|
||||
@@ -38,6 +39,12 @@
|
||||
|
||||
const failedFavicons = new SvelteSet<string>()
|
||||
|
||||
// The favicon is a request to a third party, and the public replay page promises
|
||||
// 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())
|
||||
|
||||
// Favicons come from Google's public favicon service, which discloses each
|
||||
// consulted hostname to a third party from the user's browser — an accepted
|
||||
// tradeoff for now (blocked/air-gapped environments degrade to the Globe
|
||||
@@ -62,7 +69,7 @@
|
||||
title={source.url}
|
||||
class="flex items-center gap-2 py-1 px-1.5 rounded hover:bg-surface-hover min-w-0"
|
||||
>
|
||||
{#if failedFavicons.has(hostname)}
|
||||
{#if noFavicons || failedFavicons.has(hostname)}
|
||||
<Globe class="w-3.5 h-3.5 shrink-0 text-tertiary" />
|
||||
{:else}
|
||||
<img
|
||||
|
||||
Reference in New Issue
Block a user