mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 00:03:00 +00:00
fix: address CI review nits on decision output
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
50ef72d7c8
commit
da78ec86f6
@@ -136,8 +136,34 @@ pub async fn run_systemone(
|
||||
Ok(DecisionResult {
|
||||
output: parsed.answers,
|
||||
model: parsed.model,
|
||||
// Dropped when empty, as an agent's is, so `usage` is either counted or absent.
|
||||
usage: parsed
|
||||
.usage
|
||||
.map(|u| TokenUsage::from_input_output(u.input_tokens, u.output_tokens)),
|
||||
.map(|u| TokenUsage::from_input_output(u.input_tokens, u.output_tokens))
|
||||
.filter(|u| !u.is_empty()),
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use serde_json::json;
|
||||
|
||||
/// Refused are the values a flow produces when it forgot the input: no key, `null`, or a blank
|
||||
/// string from an empty expression. Any other value, falsy ones included, is a state.
|
||||
#[test]
|
||||
fn decision_inputs_refuse_only_what_a_flow_forgot() {
|
||||
let questions = json!({"urgent": {"type": "noul", "instructions": "Is it urgent?"}});
|
||||
for state in [None, Some(json!(null)), Some(json!(" "))] {
|
||||
let err = decision_inputs(state.as_ref(), Some(&questions)).unwrap_err();
|
||||
assert!(err.to_string().contains("'state'"), "{err}");
|
||||
}
|
||||
for state in [json!(false), json!("0"), json!({}), json!([]), json!({"m": "hi"})] {
|
||||
assert!(decision_inputs(Some(&state), Some(&questions)).is_ok(), "{state}");
|
||||
}
|
||||
for questions in [None, Some(json!({})), Some(json!([])), Some(json!("urgent"))] {
|
||||
let err = decision_inputs(Some(&json!("hi")), questions.as_ref()).unwrap_err();
|
||||
assert!(err.to_string().contains("'questions'"), "{err}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -725,6 +725,19 @@ pub async fn handle_ai_agent_job(
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
let ignored = ignored_by_decision(&args, !tools.is_empty());
|
||||
if !ignored.is_empty() {
|
||||
append_logs(
|
||||
&job.id,
|
||||
&job.workspace_id,
|
||||
format!(
|
||||
"Decision output reads only provider, state and questions, so it ignores the step's {}.\n",
|
||||
ignored.join(", ")
|
||||
),
|
||||
conn,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
// A decision is one call that offers no tools, so its roster is never resolved: an MCP
|
||||
// server the step still lists is not contacted, and there is nothing a narrowing could name.
|
||||
args.enabled_tools = None;
|
||||
@@ -2097,6 +2110,38 @@ pub async fn run_agent(
|
||||
}))
|
||||
}
|
||||
|
||||
/// The settings a decision step holds that its run never reads. A step switched from text output
|
||||
/// keeps them, and so does one linked to an agent that was, whose form no longer shows them.
|
||||
fn ignored_by_decision(args: &AIAgentArgs, has_tools: bool) -> Vec<&'static str> {
|
||||
let non_empty = |s: &Option<String>| s.as_deref().is_some_and(|s| !s.trim().is_empty());
|
||||
[
|
||||
("tools", has_tools),
|
||||
("enabled_tools", args.enabled_tools.is_some()),
|
||||
("system_prompt", non_empty(&args.system_prompt)),
|
||||
("user_message", non_empty(&args.user_message)),
|
||||
(
|
||||
"user_attachments",
|
||||
args.user_attachments.as_ref().is_some_and(|a| !a.is_empty()),
|
||||
),
|
||||
("memory", !matches!(args.memory, None | Some(Memory::Off))),
|
||||
("memory_id", args.memory_id.is_some()),
|
||||
("previous_messages", args.previous_messages.is_some()),
|
||||
(
|
||||
"output_schema",
|
||||
args.output_schema
|
||||
.as_ref()
|
||||
.and_then(|schema| schema.properties.as_ref())
|
||||
.is_some_and(|properties| !properties.is_empty()),
|
||||
),
|
||||
("temperature", args.temperature.is_some()),
|
||||
("max_completion_tokens", args.max_completion_tokens.is_some()),
|
||||
("max_iterations", args.max_iterations.is_some()),
|
||||
]
|
||||
.into_iter()
|
||||
.filter_map(|(name, set)| set.then_some(name))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Decision output: the step's questions answered about its state in a single call, with no loop,
|
||||
/// tools, memory or stream.
|
||||
async fn run_decision(
|
||||
|
||||
@@ -64,6 +64,21 @@
|
||||
}
|
||||
}
|
||||
|
||||
// A kind the mode does not offer cannot run, and switching the output type leaves the old kind
|
||||
// behind. Its resource, model and reasoning effort belong to it, so they go with it rather than
|
||||
// surfacing only as a failed run.
|
||||
$effect(() => {
|
||||
const offered = offeredProviders
|
||||
untrack(() => {
|
||||
if (value?.kind && !offered.includes(value.kind)) {
|
||||
value.kind = decision ? 'typesafe' : 'openai'
|
||||
value.resource = ''
|
||||
value.model = ''
|
||||
value.reasoning_effort = undefined
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
let useAsDefault = $derived(isSameAsStoredConfig(value))
|
||||
|
||||
// Reactive items for the Select component
|
||||
|
||||
@@ -25,6 +25,8 @@
|
||||
openFieldsKey?: string
|
||||
/** Passed through to the form: fields it must offer whatever the step holds. */
|
||||
runInputKeys?: readonly string[]
|
||||
/** Passed through to the form: a linked agent's output type. */
|
||||
linkedOutputType?: unknown
|
||||
}
|
||||
|
||||
let {
|
||||
@@ -40,7 +42,8 @@
|
||||
onJobDone,
|
||||
hideRunButton = false,
|
||||
openFieldsKey = undefined,
|
||||
runInputKeys = undefined
|
||||
runInputKeys = undefined,
|
||||
linkedOutputType = undefined
|
||||
}: Props = $props()
|
||||
|
||||
const { flowStore } = getContext<FlowEditorContext>('FlowEditorContext')
|
||||
@@ -91,5 +94,13 @@
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
<ModulePreviewForm {pickableProperties} {mod} {schema} {focusArg} {openFieldsKey} {runInputKeys} />
|
||||
<ModulePreviewForm
|
||||
{pickableProperties}
|
||||
{mod}
|
||||
{schema}
|
||||
{focusArg}
|
||||
{openFieldsKey}
|
||||
{runInputKeys}
|
||||
{linkedOutputType}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -36,6 +36,8 @@
|
||||
/** Fields to offer whatever the step holds, for a surface where nothing else can set them
|
||||
* (`AGENT_EDITOR_RUN_INPUTS`). */
|
||||
runInputKeys?: readonly string[]
|
||||
/** A linked agent's `output_type`, once its config has loaded: the step holds none of its own. */
|
||||
linkedOutputType?: unknown
|
||||
}
|
||||
|
||||
let {
|
||||
@@ -46,7 +48,8 @@
|
||||
autofocus = false,
|
||||
focusArg = undefined,
|
||||
openFieldsKey = undefined,
|
||||
runInputKeys = []
|
||||
runInputKeys = [],
|
||||
linkedOutputType = undefined
|
||||
}: Props = $props()
|
||||
|
||||
const { stepsInputArgs, flowStateStore, flowStore, previewArgs, opWorkspace } =
|
||||
@@ -76,18 +79,21 @@
|
||||
if ((mod.value as { type?: string })?.type !== 'aiagent') return all
|
||||
const transforms = (mod.value as { input_transforms?: Record<string, unknown> })
|
||||
?.input_transforms
|
||||
const visible = initialVisibleAgentFields(transforms, schema?.properties)
|
||||
for (const key of openAgentFields(openFieldsKey)) visible.add(key)
|
||||
for (const key of runInputKeys) visible.add(key)
|
||||
// The open fields can carry the other output type's from before a switch. Only a step's own
|
||||
// agent says which output type it runs: a linked one takes it from the agent.
|
||||
// A step's own agent holds its output type; a linked one takes it from the agent, and until
|
||||
// that has loaded no field is dropped for it.
|
||||
const outputTransform = transforms?.output_type as
|
||||
| { type?: string; value?: unknown }
|
||||
| undefined
|
||||
if (outputTransform) {
|
||||
const outputType = agentOutputType(
|
||||
outputTransform.type === 'static' ? outputTransform.value : undefined
|
||||
)
|
||||
const outputType = outputTransform
|
||||
? agentOutputType(outputTransform.type === 'static' ? outputTransform.value : undefined)
|
||||
: linkedOutputType !== undefined
|
||||
? agentOutputType(linkedOutputType)
|
||||
: undefined
|
||||
const visible = initialVisibleAgentFields(transforms, schema?.properties, outputType)
|
||||
for (const key of openAgentFields(openFieldsKey)) visible.add(key)
|
||||
for (const key of runInputKeys) visible.add(key)
|
||||
// The open fields can carry the other output type's from before a switch.
|
||||
if (outputType) {
|
||||
for (const spec of AGENT_FIELDS) {
|
||||
if (!agentFieldServes(spec, outputType)) visible.delete(spec.key)
|
||||
}
|
||||
|
||||
@@ -67,13 +67,7 @@ describe('validateAiAgentProviders', () => {
|
||||
expect(() =>
|
||||
validateAiAgentProviders(anthropicProvider('claude-something-new'), {
|
||||
...COMPLETE,
|
||||
options: [
|
||||
{
|
||||
...ANTHROPIC,
|
||||
modelsAreLive: false,
|
||||
models: { ids: ['claude-sonnet-5'], complete: false }
|
||||
}
|
||||
]
|
||||
options: [{ ...ANTHROPIC, modelsAreLive: false, models: { ids: ['claude-sonnet-5'], complete: false } }]
|
||||
})
|
||||
).not.toThrow()
|
||||
})
|
||||
@@ -182,12 +176,10 @@ describe('formatAiAgentProvidersPrompt', () => {
|
||||
expect(
|
||||
formatAiAgentProvidersPrompt({ ...COMPLETE, defaultModel }, { canAskUser: true })
|
||||
).toContain('the workspace default')
|
||||
expect(
|
||||
formatAiAgentProvidersPrompt({ ...AMBIGUOUS, defaultModel }, { canAskUser: true })
|
||||
).toContain('askUserQuestion')
|
||||
expect(formatAiAgentProvidersPrompt(COMPLETE, { canAskUser: true })).toContain(
|
||||
expect(formatAiAgentProvidersPrompt({ ...AMBIGUOUS, defaultModel }, { canAskUser: true })).toContain(
|
||||
'askUserQuestion'
|
||||
)
|
||||
expect(formatAiAgentProvidersPrompt(COMPLETE, { canAskUser: true })).toContain('askUserQuestion')
|
||||
})
|
||||
|
||||
it('never names askUserQuestion for a chat that does not have the tool', () => {
|
||||
@@ -199,10 +191,9 @@ describe('formatAiAgentProvidersPrompt', () => {
|
||||
|
||||
describe('collectAiAgentProviderRefs', () => {
|
||||
it('needs the catalog only for a step that states its own static provider', () => {
|
||||
expect(collectAiAgentProviderRefs(anthropicProvider('claude-sonnet-5'))).toEqual({
|
||||
needsCatalog: true,
|
||||
resourceRefs: ['$res:u/admin/anthropic']
|
||||
})
|
||||
expect(
|
||||
collectAiAgentProviderRefs(anthropicProvider('claude-sonnet-5'))
|
||||
).toEqual({ needsCatalog: true, resourceRefs: ['$res:u/admin/anthropic'] })
|
||||
expect(
|
||||
collectAiAgentProviderRefs([{ id: 'a', value: { type: 'aiagent', agent: 'u/admin/saved' } }])
|
||||
).toEqual({ needsCatalog: false, resourceRefs: [] })
|
||||
@@ -224,10 +215,7 @@ describe('an authoritative empty catalog', () => {
|
||||
)
|
||||
// Not knowing the resources is not the same as knowing there are none.
|
||||
expect(
|
||||
formatAiAgentProvidersPrompt(
|
||||
{ options: [], resourcesAreComplete: false },
|
||||
{ canAskUser: true }
|
||||
)
|
||||
formatAiAgentProvidersPrompt({ options: [], resourcesAreComplete: false }, { canAskUser: true })
|
||||
).toBe('')
|
||||
})
|
||||
|
||||
|
||||
@@ -177,9 +177,7 @@ export function selectAiAgentProviderCandidates(
|
||||
if (!configuredPaths.has(candidate.resourcePath)) return 2
|
||||
return candidate.kind === defaultProviderKind ? 0 : 1
|
||||
}
|
||||
return candidates.sort(
|
||||
(a, b) => rank(a) - rank(b) || a.resourcePath.localeCompare(b.resourcePath)
|
||||
)
|
||||
return candidates.sort((a, b) => rank(a) - rank(b) || a.resourcePath.localeCompare(b.resourcePath))
|
||||
}
|
||||
|
||||
/** What a set of modules needs from the catalog: `needsCatalog` is false when no AI agent step
|
||||
|
||||
@@ -1350,6 +1350,7 @@
|
||||
{onJobDone}
|
||||
hideRunButton={debugMode && isDebuggableScript}
|
||||
openFieldsKey={agentFieldsKey}
|
||||
linkedOutputType={agentLinked ? linkedAgentBrain?.output_type : undefined}
|
||||
/>
|
||||
{:else if visibleSelected === 'chat' && canShowChatTab && flowModule.value.type === 'aiagent'}
|
||||
<div class="flex-1 overflow-auto p-4">
|
||||
|
||||
Reference in New Issue
Block a user