mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: bypass memory when a provided messages expression evaluates to null
This commit is contained in:
@@ -94,6 +94,12 @@ pub enum Memory {
|
||||
},
|
||||
}
|
||||
|
||||
fn deserialize_present_messages<'de, D: serde::Deserializer<'de>>(
|
||||
deserializer: D,
|
||||
) -> Result<Option<Option<Vec<OpenAIMessage>>>, D::Error> {
|
||||
<Option<Vec<OpenAIMessage>> as serde::Deserialize>::deserialize(deserializer).map(Some)
|
||||
}
|
||||
|
||||
fn deserialize_present<'de, D: serde::Deserializer<'de>>(
|
||||
deserializer: D,
|
||||
) -> Result<Option<serde_json::Value>, D::Error> {
|
||||
@@ -118,7 +124,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>,
|
||||
messages: Option<Vec<OpenAIMessage>>,
|
||||
// Same distinction for an authored messages expression: null sends no history, and memory stays
|
||||
// bypassed rather than being read and overwritten.
|
||||
#[serde(default, deserialize_with = "deserialize_present_messages")]
|
||||
messages: Option<Option<Vec<OpenAIMessage>>>,
|
||||
// Legacy field for backward compatibility
|
||||
messages_context_length: Option<usize>,
|
||||
#[serde(default)]
|
||||
@@ -181,7 +190,7 @@ impl From<AIAgentArgsRaw> for AIAgentArgs {
|
||||
max_iterations: raw.max_iterations,
|
||||
memory,
|
||||
memory_id,
|
||||
messages: raw.messages,
|
||||
messages: raw.messages.map(Option::unwrap_or_default),
|
||||
credentials_check: raw.credentials_check.unwrap_or(false),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,8 +120,8 @@ enum HistorySource<'a> {
|
||||
}
|
||||
|
||||
/// The step's history inputs count only as the step authored them. A static empty value is a form
|
||||
/// placeholder, so a memory id reads as unset rather than as one that evaluated to nothing; an
|
||||
/// AI-filled value would let the model choose which memory the agent reads.
|
||||
/// placeholder, so it reads as unset rather than as an expression that evaluated to nothing, which
|
||||
/// runs without memory; an AI-filled value would let the model choose which memory the agent reads.
|
||||
fn keep_authored_history_args(
|
||||
args: &mut AIAgentArgs,
|
||||
step_input_transforms: &HashMap<String, InputTransform>,
|
||||
@@ -131,11 +131,10 @@ fn keep_authored_history_args(
|
||||
Some(InputTransform::Static { .. }) if args.memory_id.as_deref() != Some("") => {}
|
||||
_ => args.memory_id = None,
|
||||
}
|
||||
if !matches!(
|
||||
step_input_transforms.get("messages"),
|
||||
Some(InputTransform::Static { .. } | InputTransform::Javascript { .. })
|
||||
) {
|
||||
args.messages = None;
|
||||
match step_input_transforms.get("messages") {
|
||||
Some(InputTransform::Javascript { .. }) => {}
|
||||
Some(InputTransform::Static { value }) if value.get().trim() != "null" => {}
|
||||
_ => args.messages = None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1930,6 +1929,47 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Provided messages bypass memory even when their expression evaluates to null; only a static
|
||||
/// placeholder leaves the step on its memory.
|
||||
#[test]
|
||||
fn a_messages_expression_evaluating_to_null_bypasses_memory() {
|
||||
let run = Uuid::from_u128(1);
|
||||
for (transform, expected) in [
|
||||
(
|
||||
r#"{ "type": "javascript", "expr": "flow_input.history" }"#,
|
||||
Resolved::Messages(0),
|
||||
),
|
||||
(
|
||||
r#"{ "type": "static", "value": null }"#,
|
||||
Resolved::Window(run, 10),
|
||||
),
|
||||
(
|
||||
r#"{ "type": "static", "value": [] }"#,
|
||||
Resolved::Messages(0),
|
||||
),
|
||||
] {
|
||||
let mut args: AIAgentArgs = serde_json::from_value(serde_json::json!({
|
||||
"provider": { "kind": "openai", "resource": {}, "model": "m" },
|
||||
"memory": { "kind": "window", "context_length": 10 },
|
||||
"messages": if transform.contains("[]") { serde_json::json!([]) } else { serde_json::Value::Null },
|
||||
}))
|
||||
.unwrap();
|
||||
let transforms = HashMap::from([(
|
||||
"messages".to_string(),
|
||||
serde_json::from_str(transform).unwrap(),
|
||||
)]);
|
||||
keep_authored_history_args(&mut args, &transforms);
|
||||
let resolved = match resolve_history_source(&args, Some(run), "ws", "f/flow") {
|
||||
(HistorySource::Messages(m), _) => Resolved::Messages(m.len()),
|
||||
(HistorySource::Window { memory_id, context_length }, _) => {
|
||||
Resolved::Window(memory_id, context_length)
|
||||
}
|
||||
(HistorySource::Stateless, note) => Resolved::Stateless { noted: note.is_some() },
|
||||
};
|
||||
assert_eq!(resolved, expected, "{transform}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unwritten_streaming_field_streams() {
|
||||
assert!(streaming_requested(None));
|
||||
|
||||
Generated
+1
-1
File diff suppressed because one or more lines are too long
@@ -52,7 +52,7 @@ which memory it is:
|
||||
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. `messages` supplies the history itself and
|
||||
bypasses memory. The editor writes at most one of them and never seeds a placeholder for
|
||||
bypasses memory; an expression that evaluates to null sends no history and still bypasses it. The editor writes at most one of them and never seeds a placeholder for
|
||||
either, because a present key is the step's choice; if both are present, `messages` wins.
|
||||
|
||||
The worker reconciles them once per agent invocation, nested agent tools included, in
|
||||
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -1092,8 +1092,9 @@ components:
|
||||
- $ref: '#/components/schemas/InputTransform'
|
||||
description: |
|
||||
Array of MemoryMessage. History supplied by the flow, sent between the system prompt
|
||||
and the user message. Memory is neither read nor written. Takes precedence over
|
||||
`memory_id` and `memory`.
|
||||
and the user message. Memory is neither read nor written, also when an expression
|
||||
evaluates to null, which sends no history. Takes precedence over `memory_id` and
|
||||
`memory`.
|
||||
output_schema:
|
||||
allOf:
|
||||
- $ref: '#/components/schemas/InputTransform'
|
||||
|
||||
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
Reference in New Issue
Block a user