fix: rename step messages to previous_messages and address review

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
hugocasa
2026-09-15 20:22:05 +02:00
co-authored by Claude Opus 5
parent ad15cb740b
commit 88d9bb00eb
22 changed files with 124 additions and 79 deletions
+5 -5
View File
@@ -88,7 +88,7 @@ pub enum Memory {
#[serde(default, deserialize_with = "deserialize_blank_as_none")]
memory_id: Option<Uuid>,
},
/// Written before the step's `messages` input, which it is equivalent to.
/// Written before the step's `previous_messages` input, which it is equivalent to.
Manual {
messages: Vec<OpenAIMessage>,
},
@@ -137,10 +137,10 @@ struct AIAgentArgsRaw {
// nothing runs stateless instead of falling back to the run's memory id.
#[serde(default, deserialize_with = "deserialize_present")]
memory_id: Option<serde_json::Value>,
// Same distinction for an authored messages expression: null replaces a legacy manual list with
// Same distinction for an authored previous messages expression: null replaces a legacy manual list with
// no history, where an absent key keeps that list.
#[serde(default, deserialize_with = "deserialize_present_messages")]
messages: Option<Option<Vec<OpenAIMessage>>>,
previous_messages: Option<Option<Vec<OpenAIMessage>>>,
enabled_tools: Option<Vec<String>>,
// Legacy field for backward compatibility
messages_context_length: Option<usize>,
@@ -165,7 +165,7 @@ pub struct AIAgentArgs {
/// Memory id set on the step, overriding the run's. Empty when its expression produced none.
pub memory_id: Option<String>,
/// History supplied by the flow, replayed without reading or writing memory.
pub messages: Option<Vec<OpenAIMessage>>,
pub previous_messages: Option<Vec<OpenAIMessage>>,
/// Which of the agent's tools this run may call; `narrow_roster` holds what the names are and
/// what `None` means.
pub enabled_tools: Option<Vec<String>>,
@@ -207,7 +207,7 @@ impl From<AIAgentArgsRaw> for AIAgentArgs {
max_iterations: raw.max_iterations,
memory,
memory_id,
messages: raw.messages.map(Option::unwrap_or_default),
previous_messages: raw.previous_messages.map(Option::unwrap_or_default),
enabled_tools: raw.enabled_tools,
credentials_check: raw.credentials_check.unwrap_or(false),
}
+1 -1
View File
@@ -1096,7 +1096,7 @@ pub enum FlowModuleValue {
/// When set, the agent brain config (provider/model/system prompt/etc.) and tools are
/// resolved at runtime from this `ai_agent` resource path (hybrid linking). The module's
/// `input_transforms` then only carry the flow-local inputs: user_message,
/// user_attachments and the history inputs memory_id and messages.
/// user_attachments, enabled_tools and the history inputs memory_id and previous_messages.
#[serde(default, skip_serializing_if = "Option::is_none")]
agent: Option<String>,
/// Binds an agent's tools to *this* flow's context, keyed by tool id then input key, without
+69 -31
View File
@@ -131,17 +131,20 @@ fn keep_authored_history_args(
Some(InputTransform::Static { .. }) if args.memory_id.as_deref() != Some("") => {}
_ => args.memory_id = None,
}
match step_input_transforms.get("messages") {
match step_input_transforms.get("previous_messages") {
Some(InputTransform::Javascript { .. }) => {}
Some(InputTransform::Static { .. })
if args.messages.as_ref().is_some_and(|m| !m.is_empty()) => {}
_ => args.messages = None,
if args
.previous_messages
.as_ref()
.is_some_and(|m| !m.is_empty()) => {}
_ => args.previous_messages = None,
}
}
/// Reconciles the step's history inputs, the agent's memory policy and the run's memory id, for
/// every shape a flow or agent resource may still carry. Managed memory reads only a memory id, and
/// memory that is off reads only messages. Also returns lines for the job log: a step input that
/// memory that is off reads only previous messages. Also returns lines for the job log: a step input that
/// went unused, or a policy that remembers ending up stateless.
fn resolve_history_source<'a>(
args: &'a AIAgentArgs,
@@ -158,9 +161,10 @@ fn resolve_history_source<'a>(
if args.memory_id.as_deref().is_some_and(|id| !id.is_empty()) {
notes.push("Managed memory is off, so this step's memory id is ignored.");
}
let history = match (&args.messages, &args.memory) {
let history = match (&args.previous_messages, &args.memory) {
(Some(messages), _) => HistorySource::Messages(messages),
// The fixed list an older editor stored in `memory`, which the step's messages replace.
// The fixed list an older editor stored in `memory`, which the step's previous messages
// replace.
(None, Some(Memory::Manual { messages })) => HistorySource::Messages(messages),
_ => HistorySource::Stateless,
};
@@ -168,11 +172,11 @@ fn resolve_history_source<'a>(
}
};
if args
.messages
.previous_messages
.as_ref()
.is_some_and(|messages| !messages.is_empty())
{
notes.push("Managed memory is on, so this step's messages are ignored.");
notes.push("Managed memory is on, so this step's previous messages are ignored.");
}
let memory_id = match args.memory_id.as_deref() {
Some("") => {
@@ -194,6 +198,14 @@ fn resolve_history_source<'a>(
(HistorySource::Window { memory_id, context_length }, notes)
}
/// Whether a request has something to ask the model. Only text output sends previous messages, so an
/// image prompt comes from the user message alone. An empty list, as an expression evaluating to
/// null gives, is no conversation.
fn has_prompt(history: &HistorySource, has_user_message: bool, is_text_output: bool) -> bool {
has_user_message
|| (is_text_output && matches!(history, HistorySource::Messages(m) if !m.is_empty()))
}
fn find_module_by_id(
modules: &Vec<FlowModule>,
target_id: &str,
@@ -555,8 +567,9 @@ pub async fn handle_ai_agent_job(
// A linked step takes its brain and tools from the resource and keeps only its own flow-local
// inputs. The brain and the roster stay rigid; what the step binds to this flow is the message
// it asks, which of those tools this use may call, the conversation it is part of (its memory id
// and messages), and the tools' own inputs — the last overlaid from `tool_inputs` below.
// it asks, which of those tools this use may call, the conversation it is part of (its memory
// id and previous messages), and the tools' own inputs — the last overlaid from `tool_inputs`
// below.
let (mut args, tools): (AIAgentArgs, Vec<AgentTool>) = if let Some(agent_ref) = agent.as_deref()
{
let agent_path = agent_ref
@@ -616,7 +629,7 @@ pub async fn handle_ai_agent_job(
"user_attachments",
"enabled_tools",
"memory_id",
"messages",
"previous_messages",
] {
if let Some(v) = local_args.get(key) {
brain.insert(
@@ -1128,19 +1141,29 @@ pub async fn run_agent(
.map(|m| !m.is_empty())
.unwrap_or(false);
// An empty list, as a messages expression evaluating to null gives, is no conversation to send.
if !matches!(&history, HistorySource::Messages(m) if !m.is_empty()) && !has_user_message {
return Err(Error::internal_err(
"Either 'messages' or 'user_message' must be provided".to_string(),
));
}
let is_text_output = output_type == &OutputType::Text;
if matches!(output_type, OutputType::Text) {
for note in history_notes {
if is_text_output {
for note in &history_notes {
append_logs(&job.id, &job.workspace_id, format!("{note}\n"), conn).await;
}
}
if !has_prompt(&history, has_user_message, is_text_output) {
let missing = if !is_text_output {
"'user_message' must be provided for image output"
} else if matches!(
args.memory,
Some(Memory::Window { .. } | Memory::Auto { .. })
) {
"'user_message' must be provided while managed memory is on"
} else {
"Either 'previous_messages' or 'user_message' must be provided"
};
return Err(Error::internal_err(missing.to_string()));
}
if matches!(output_type, OutputType::Text) {
match &history {
HistorySource::Messages(provided) => messages.extend(provided.iter().cloned()),
HistorySource::Window { memory_id, context_length } => {
@@ -2039,20 +2062,20 @@ mod tests {
Resolved::Stateless { noted: true },
),
(
"managed memory ignores the step's messages",
json!({ "memory": window, "memory_id": "cust_1", "messages": message }),
"managed memory ignores the step's previous messages",
json!({ "memory": window, "memory_id": "cust_1", "previous_messages": message }),
Some(run),
Resolved::Window(cust_1, 10),
),
(
"memory that is off sends the step's messages",
json!({ "messages": message }),
"memory that is off sends the step's previous messages",
json!({ "previous_messages": message }),
Some(run),
Resolved::Messages(1),
),
(
"the step's messages replace a legacy manual list",
json!({ "memory": { "kind": "manual", "messages": message }, "messages": two_messages }),
"the step's previous messages replace a legacy manual list",
json!({ "memory": { "kind": "manual", "messages": message }, "previous_messages": two_messages }),
Some(run),
Resolved::Messages(2),
),
@@ -2090,7 +2113,7 @@ mod tests {
serde_json::from_value(serde_json::json!({
"provider": { "kind": "openai", "resource": {}, "model": "m" },
"memory_id": null,
"messages": [],
"previous_messages": [],
}))
.unwrap()
};
@@ -2106,11 +2129,11 @@ mod tests {
let mut args = args();
keep_authored_history_args(&mut args, &transforms(transform));
assert_eq!(args.memory_id.as_deref(), expected, "{transform}");
assert!(args.messages.is_none(), "{transform}");
assert!(args.previous_messages.is_none(), "{transform}");
}
}
/// Empty messages a form leaves on a step never replace a legacy list; an expression does, even
/// Empty previous messages a form leaves on a step never replace a legacy list; an expression does, even
/// when it evaluates to null.
#[test]
fn only_an_expression_can_empty_a_legacy_message_list() {
@@ -2132,11 +2155,11 @@ mod tests {
let mut args: AIAgentArgs = serde_json::from_value(serde_json::json!({
"provider": { "kind": "openai", "resource": {}, "model": "m" },
"memory": { "kind": "manual", "messages": [{ "role": "user", "content": "earlier" }] },
"messages": if transform.contains("[]") { serde_json::json!([]) } else { serde_json::Value::Null },
"previous_messages": if transform.contains("[]") { serde_json::json!([]) } else { serde_json::Value::Null },
}))
.unwrap();
let transforms = HashMap::from([(
"messages".to_string(),
"previous_messages".to_string(),
serde_json::from_str(transform).unwrap(),
)]);
keep_authored_history_args(&mut args, &transforms);
@@ -2153,6 +2176,21 @@ mod tests {
}
}
/// Only text output sends previous messages, so they never stand in for an image prompt.
#[test]
fn previous_messages_never_stand_in_for_an_image_prompt() {
let args: AIAgentArgs = serde_json::from_value(serde_json::json!({
"provider": { "kind": "openai", "resource": {}, "model": "m" },
"previous_messages": [{ "role": "user", "content": "earlier" }],
}))
.unwrap();
let (history, _) = resolve_history_source(&args, None, "ws", "f/flow");
assert!(has_prompt(&history, false, true));
assert!(!has_prompt(&history, false, false));
assert!(has_prompt(&history, true, false));
assert!(!has_prompt(&HistorySource::Messages(&[]), false, true));
}
#[test]
fn reasoning_keeps_every_iteration_in_order() {
let mut acc = String::new();
+1 -1
View File
File diff suppressed because one or more lines are too long
+4 -4
View File
@@ -17,7 +17,7 @@ every workspace via the standard cached-resource-type sync, like other built-in
(`windmill-worker/src/ai_executor.rs`): the brain is interpolated, so a nested provider `$res:`
credential resolves automatically.
- The step keeps only the flow-local inputs (`user_message`, `user_attachments`, `enabled_tools`,
and the history inputs `memory_id` and `messages`) in its own `input_transforms`; the brain and
and the history inputs `memory_id` and `previous_messages`) in its own `input_transforms`; the brain and
tools stay in the resource (read-only in the step). `enabled_tools` says which of the roster this
step may call, narrowing one use of a shared agent without touching the agent: an absent field
carries every tool, a list carries the ones it names, and an empty list carries none.
@@ -56,16 +56,16 @@ which memory it is:
writes the key only once it is on. With managed memory on, `memory_id` overrides
the run's id, hashed the same way: a fixed value is one memory shared by every run, an expression
such as `flow_input.customer_id` one memory per key, and an expression that evaluates to nothing
runs stateless rather than falling back to the run's id. With memory off, `messages` supplies the
runs stateless rather than falling back to the run's id. With memory off, `previous_messages` supplies the
history itself. The editor never seeds a placeholder for either, because a present key is the
step's choice, and a static empty value reads as unset.
The worker reconciles them once per agent invocation, nested agent tools included, in
`resolve_history_source` (`windmill-worker/src/ai_executor.rs`):
1. Memory off, or a legacy `manual` memory: the history is `messages`, else the `manual` list,
1. Memory off, or a legacy `manual` memory: the history is `previous_messages`, else the `manual` list,
else nothing. Memory is neither read nor written, and a step `memory_id` is ignored.
2. Managed memory: a step `messages` is ignored. The memory id is the step's, else the run's, else
2. Managed memory: a step `previous_messages` is ignored. The memory id is the step's, else the run's, else
a legacy id baked into the `auto` object. With no memory id the agent runs stateless.
Each ignored input and each stateless fallback is written to the job log.
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -88,16 +88,16 @@ describe('initialVisibleAgentFields', () => {
describe('historyInputApplies', () => {
// Mirrors the worker: offering a step input a run would ignore misleads the author.
it('offers a memory id only with managed memory, and messages only without', () => {
it('offers a memory id only with managed memory, and previous messages only without', () => {
expect(keepsManagedMemory(undefined)).toBe(false)
expect(keepsManagedMemory({ kind: 'window', context_length: 0 })).toBe(false)
expect(keepsManagedMemory({ kind: 'manual', messages: [] })).toBe(false)
expect(keepsManagedMemory({ kind: 'auto', context_length: 4, memory_id: 'x' })).toBe(true)
expect(historyInputApplies('memory_id', true)).toBe(true)
expect(historyInputApplies('messages', true)).toBe(false)
expect(historyInputApplies('previous_messages', true)).toBe(false)
expect(historyInputApplies('memory_id', false)).toBe(false)
expect(historyInputApplies('messages', false)).toBe(true)
expect(historyInputApplies('messages', undefined)).toBe(true)
expect(historyInputApplies('previous_messages', false)).toBe(true)
expect(historyInputApplies('previous_messages', undefined)).toBe(true)
})
})
@@ -27,7 +27,7 @@ export const AGENT_TOOLS_ROW = 'tools'
/** A step's own history inputs. Never seeded with a placeholder: a run reads a present key as the
* step's choice, so only the author adds them. */
export const AGENT_HISTORY_KEYS = ['memory_id', 'messages'] as const
export const AGENT_HISTORY_KEYS = ['memory_id', 'previous_messages'] as const
export type AgentHistoryKey = (typeof AGENT_HISTORY_KEYS)[number]
/** What turning managed memory on writes. */
@@ -44,7 +44,7 @@ export function keepsManagedMemory(memory: any): boolean {
}
/** Whether a run reads this step input, mirroring the worker: managed memory reads only a memory id,
* memory that is off only messages. A setting the form cannot read yet leaves both open. */
* memory that is off only previous messages. A setting the form cannot read yet leaves both open. */
export function historyInputApplies(
key: AgentHistoryKey,
managedMemory: boolean | undefined
@@ -145,7 +145,7 @@ export const AGENT_FIELDS: AgentFieldSpec[] = [
textOnly: true
},
{
key: 'messages',
key: 'previous_messages',
group: 'messages',
label: 'Previous messages',
tooltip: 'History the flow supplies, sent between the system message and the user message.',
@@ -151,7 +151,7 @@ describe('flowLocalInputs', () => {
user_message: { type: 'static', value: 'hi' },
user_attachments: { type: 'static', value: [] },
memory_id: { type: 'javascript', expr: 'flow_input.customer_id' },
messages: { type: 'static', value: [{ role: 'user', content: 'earlier' }] },
previous_messages: { type: 'static', value: [{ role: 'user', content: 'earlier' }] },
// The roster it narrows belongs to the agent, but which of it one flow may call does
// not: saving this into the resource would impose it on every flow linking the agent.
enabled_tools: { type: 'javascript', expr: 'flow_input.tools' }
@@ -160,7 +160,7 @@ describe('flowLocalInputs', () => {
user_message: { type: 'static', value: 'hi' },
user_attachments: { type: 'static', value: [] },
memory_id: { type: 'javascript', expr: 'flow_input.customer_id' },
messages: { type: 'static', value: [{ role: 'user', content: 'earlier' }] },
previous_messages: { type: 'static', value: [{ role: 'user', content: 'earlier' }] },
enabled_tools: { type: 'javascript', expr: 'flow_input.tools' }
})
})
@@ -28,7 +28,7 @@ export const AGENT_FLOW_LOCAL_KEYS = [
'user_attachments',
'enabled_tools',
'memory_id',
'messages'
'previous_messages'
] as const
export type AgentTool = Record<string, any>
@@ -7,7 +7,7 @@
* it moves into. */
args: Record<string, any>
chatInputEnabled?: boolean
/** Whether the step's own memory id and messages are on this form. A saved agent has neither:
/** Whether the step's own memory id and previous messages are on this form. A saved agent has neither:
* they belong to each step linking it. */
historyOnStep?: boolean
s3StorageConfigured?: boolean
@@ -31,9 +31,14 @@
let legacyMessages = $derived(
memory?.kind === 'manual' ? ((memory.messages ?? []) as unknown[]) : undefined
)
// A chat run always carries the conversation's memory id, so there a baked id was never read.
// A chat run always carries the conversation's memory id, and a step's own memory id replaces the
// baked one, so in either case the baked id is never read. A blank static id reads as unset.
let stepMemoryId = $derived(
args?.memory_id?.type === 'javascript' ||
(args?.memory_id?.type === 'static' && String(args.memory_id.value ?? '').trim() !== '')
)
let legacyMemoryId = $derived(
on && memory?.kind === 'auto' && memory.memory_id && !chatInputEnabled
on && memory?.kind === 'auto' && memory.memory_id && !chatInputEnabled && !stepMemoryId
? String(memory.memory_id)
: undefined
)
@@ -60,7 +65,7 @@
}
function moveMessagesToStep() {
args.messages = { type: 'static', value: $state.snapshot(legacyMessages) ?? [] }
args.previous_messages = { type: 'static', value: $state.snapshot(legacyMessages) ?? [] }
args.memory = { type: 'static', value: { kind: 'off' } }
}
</script>
@@ -552,7 +552,7 @@
<AgentMemoryNotes
bind:args
{chatInputEnabled}
historyOnStep={scopedFields.some((f) => f.key === 'messages')}
historyOnStep={scopedFields.some((f) => f.key === 'previous_messages')}
s3StorageConfigured={s3Storage.current}
/>
{:else if spec.key === 'memory_id' && memoryIdOffered}
@@ -619,10 +619,12 @@
}
// A linked step's memory belongs to the agent it links, and a step supplying its own
// messages reads them only while memory is off.
// previous messages reads them only while memory is off. An empty static list supplies none.
const messages = value.input_transforms['previous_messages']
if (
!value.agent &&
value.input_transforms['messages'] == undefined &&
(isUnconfigured(messages) ||
(messages?.type === 'static' && !(messages.value as unknown[] | undefined)?.length)) &&
isUnconfigured(value.input_transforms['memory'])
) {
value.input_transforms['memory'] = {
@@ -79,7 +79,7 @@ export const AI_AGENT_SCHEMA: Schema = {
'Names the memory this step reads and writes, overriding the memory id the run was started with. Read only while managed memory is on.',
showExpr: "fields.output_type !== 'image'"
},
messages: {
previous_messages: {
type: 'array',
description:
'History the flow supplies, sent before the user message. Read only while managed memory is off.',
@@ -175,7 +175,7 @@ export const AI_AGENT_SCHEMA: Schema = {
'streaming',
'memory',
'memory_id',
'messages',
'previous_messages',
'output_schema',
'user_attachments',
'enabled_tools',
@@ -203,7 +203,7 @@ export const LEGACY_MEMORY_VARIANTS: Record<string, any> = {
title: 'manual',
properties: {
kind: { type: 'string', enum: ['manual'] },
messages: { type: 'array', items: AI_AGENT_SCHEMA.properties?.messages?.items }
messages: { type: 'array', items: AI_AGENT_SCHEMA.properties?.previous_messages?.items }
},
required: ['kind', 'messages']
}
@@ -178,7 +178,7 @@ type AiAgentValue = Extract<FlowModule['value'], { type: 'aiagent' }>
*
* The overlay order is the worker's (`ai_executor.rs`): its linked branch interpolates the whole
* resource brain and only then writes the flow-local inputs (`user_message`, `user_attachments`,
* `memory_id`, `messages`) back from the step's own args. `tool_inputs` stays untouched the worker overlays it onto the tools in both branches, so
* `enabled_tools`, `memory_id`, `previous_messages`) back from the step's own args. `tool_inputs` stays untouched the worker overlays it onto the tools in both branches, so
* an inlined step keeps the host flow's tool bindings.
*/
export function inlineAgentDraft(value: AiAgentValue, args: AIAgentConfig): AiAgentValue {
@@ -175,9 +175,9 @@ export function normalizeAgentHistory(
if (memoryId?.type === 'static' && !String(memoryId.value ?? '').trim()) {
delete inputTransforms.memory_id
}
const messages = inputTransforms.messages
if (messages?.type === 'static' && !messages.value?.length) {
delete inputTransforms.messages
const previousMessages = inputTransforms.previous_messages
if (previousMessages?.type === 'static' && !previousMessages.value?.length) {
delete inputTransforms.previous_messages
}
}
@@ -81,7 +81,7 @@ describe('normalizeAgentHistory', () => {
it('does not persist an empty static memory id or message list', () => {
const transforms: Record<string, any> = {
memory_id: { type: 'static', value: ' ' },
messages: { type: 'static', value: [] }
previous_messages: { type: 'static', value: [] }
}
normalizeAgentHistory(transforms, false)
expect(transforms).toEqual({})
+4 -4
View File
@@ -589,7 +589,7 @@ components:
MemoryManual:
type: object
deprecated: true
description: Deprecated, still read. Use the step's `messages` input instead.
description: Deprecated, still read. Use the step's `previous_messages` input instead.
properties:
kind:
type: string
@@ -604,7 +604,7 @@ components:
- messages
MemoryConfig:
description: Managed memory, stored by Windmill and replayed with each request. The memory is named by a memory id, see `memory_id`. While it is off, a step can supply its history in `messages`.
description: Managed memory, stored by Windmill and replayed with each request. The memory is named by a memory id, see `memory_id`. While it is off, a step can supply its history in `previous_messages`.
oneOf:
- $ref: '#/components/schemas/MemoryOff'
- $ref: '#/components/schemas/MemoryWindow'
@@ -1087,7 +1087,7 @@ components:
across every run; an expression such as `flow_input.customer_id` keeps one memory per
key. When it evaluates to an empty value the agent runs without memory. Read only
while `memory` keeps messages; ignored when it is off.
messages:
previous_messages:
allOf:
- $ref: '#/components/schemas/InputTransform'
description: |
@@ -1167,7 +1167,7 @@ components:
Path of a reusable `ai_agent` resource (hybrid linking). When set, the agent brain
config (provider/model/system prompt/etc.) and tool set are resolved at runtime from
that resource; the module's input_transforms then only carry the flow-local inputs
(user_message, user_attachments, enabled_tools and the history inputs memory_id and messages).
(user_message, user_attachments, enabled_tools and the history inputs memory_id and previous_messages).
tool_inputs:
type: object
description: |
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long