From a8e2f1f1ca28e3352aff8a450188e3ff75a9637c Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 12 Oct 2026 11:45:03 +0200 Subject: [PATCH] fix: keep ai agent schema property order stable across jobs (#11687) * fix: keep ai agent schema property order stable across jobs Co-Authored-By: Claude Opus 5.5 * fix: keep tool parameter order when filtering preset inputs Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- backend/windmill-ai/src/types.rs | 45 +++++++++++++++++++------ backend/windmill-worker/src/ai/utils.rs | 35 +++++++++++++++++-- 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/backend/windmill-ai/src/types.rs b/backend/windmill-ai/src/types.rs index e591e85159..59733e352c 100644 --- a/backend/windmill-ai/src/types.rs +++ b/backend/windmill-ai/src/types.rs @@ -1,3 +1,4 @@ +use indexmap::IndexMap; use serde::{Deserialize, Serialize}; use serde_json::value::RawValue; use std::collections::HashMap; @@ -533,7 +534,7 @@ pub struct OpenAPISchema { #[serde(skip_serializing_if = "Option::is_none")] pub items: Option>, #[serde(skip_serializing_if = "Option::is_none")] - pub properties: Option>>, + pub properties: Option>>, #[serde(skip_serializing_if = "Option::is_none")] pub required: Option>, #[serde(skip_serializing_if = "Option::is_none", rename = "oneOf")] @@ -560,9 +561,9 @@ pub struct OpenAPISchema { #[serde(skip_serializing_if = "Option::is_none", rename = "$ref")] pub ref_path: Option, #[serde(skip_serializing_if = "Option::is_none", rename = "$defs")] - pub defs: Option>>, + pub defs: Option>>, #[serde(skip_serializing_if = "Option::is_none")] - pub definitions: Option>>, + pub definitions: Option>>, // Schema composition #[serde(skip_serializing_if = "Option::is_none", rename = "allOf")] @@ -829,7 +830,7 @@ impl OpenAPISchema { // Merge properties (other's properties are added if key doesn't exist) if let Some(ref other_props) = other.properties { - let props = self.properties.get_or_insert_with(HashMap::new); + let props = self.properties.get_or_insert_with(IndexMap::new); for (key, value) in other_props { props.entry(key.clone()).or_insert_with(|| value.clone()); } @@ -880,13 +881,13 @@ impl OpenAPISchema { // Merge $defs and definitions if let Some(ref other_defs) = other.defs { - let defs = self.defs.get_or_insert_with(HashMap::new); + let defs = self.defs.get_or_insert_with(IndexMap::new); for (key, value) in other_defs { defs.entry(key.clone()).or_insert_with(|| value.clone()); } } if let Some(ref other_definitions) = other.definitions { - let definitions = self.definitions.get_or_insert_with(HashMap::new); + let definitions = self.definitions.get_or_insert_with(IndexMap::new); for (key, value) in other_definitions { definitions .entry(key.clone()) @@ -981,7 +982,29 @@ pub struct S3ObjectWithType { #[cfg(test)] mod tests { use super::*; - use std::collections::HashMap; + + /// The schema is part of the prompt prefix the provider caches on, so its keys must + /// serialize in the same order on every request. + #[test] + fn strict_schema_keeps_the_defined_property_order() { + let keys = [ + "zeta", "alpha", "mid", "beta", "omega", "gamma", "delta", "kappa", + ]; + let props = keys + .iter() + .map(|k| format!(r#""{k}":{{"type":"string"}}"#)) + .collect::>() + .join(","); + let mut schema: OpenAPISchema = + serde_json::from_str(&format!(r#"{{"type":"object","properties":{{{props}}}}}"#)) + .unwrap(); + schema.make_strict(); + + let value = serde_json::to_value(&schema).unwrap(); + let serialized: Vec<&String> = value["properties"].as_object().unwrap().keys().collect(); + assert_eq!(serialized, keys); + assert_eq!(value["required"], serde_json::json!(keys)); + } /// Whichever provider served the request, `input_tokens` has to end up meaning the /// whole prompt. Leaving the Anthropic shape as reported under-states it by the @@ -1248,7 +1271,7 @@ mod tests { #[test] fn test_make_strict_defs() { let def_schema = object_schema(vec![("prop", string_schema())]); - let mut defs = HashMap::new(); + let mut defs = IndexMap::new(); defs.insert("MyType".to_string(), Box::new(def_schema)); let mut schema = OpenAPISchema { defs: Some(defs), ..Default::default() }; @@ -1274,7 +1297,7 @@ mod tests { #[test] fn test_make_strict_definitions() { let def_schema = object_schema(vec![("prop", string_schema())]); - let mut definitions = HashMap::new(); + let mut definitions = IndexMap::new(); definitions.insert("MyType".to_string(), Box::new(def_schema)); let mut schema = OpenAPISchema { definitions: Some(definitions), ..Default::default() }; @@ -1528,7 +1551,7 @@ mod tests { #[test] fn test_flatten_all_of_preserves_defs() { let def_schema = object_schema(vec![("field", string_schema())]); - let mut defs = HashMap::new(); + let mut defs = IndexMap::new(); defs.insert("MyType".to_string(), Box::new(def_schema)); let schema_with_defs = OpenAPISchema { defs: Some(defs), ..Default::default() }; @@ -1642,7 +1665,7 @@ mod tests { schema_url: Some("http://json-schema.org/draft-07/schema#".to_string()), ..Default::default() }; - let mut defs = HashMap::new(); + let mut defs = IndexMap::new(); defs.insert("MyType".to_string(), Box::new(def_schema)); let mut schema = OpenAPISchema { diff --git a/backend/windmill-worker/src/ai/utils.rs b/backend/windmill-worker/src/ai/utils.rs index 50bbcad58f..f0a0b5c20f 100644 --- a/backend/windmill-worker/src/ai/utils.rs +++ b/backend/windmill-worker/src/ai/utils.rs @@ -103,9 +103,9 @@ pub fn filter_schema_by_input_transforms( .get_mut("properties") .and_then(|p| p.as_object_mut()) { - for key in &keys_to_remove { - properties.remove(key); - } + // `remove` swaps the last entry into the freed slot, so removing in set order + // would leave the survivors in a different order on every run. + properties.retain(|key, _| !keys_to_remove.contains(key)); } // Also remove from required array @@ -748,3 +748,32 @@ pub fn any_tool_needs_previous_result(tools: &[Tool]) -> bool { false }) } + +#[cfg(test)] +mod tests { + use super::*; + + /// A tool's schema sits in the prompt prefix the provider caches on, so the + /// parameters left for the model must keep their order on every run. + #[test] + fn filtering_preset_inputs_keeps_the_remaining_property_order() { + let keys = ["a", "b", "c", "d", "e", "f", "g", "h"]; + let props = keys + .iter() + .map(|k| format!(r#""{k}":{{"type":"string"}}"#)) + .collect::>() + .join(","); + let schema = + RawValue::from_string(format!(r#"{{"type":"object","properties":{{{props}}}}}"#)) + .unwrap(); + let preset = ["a", "c", "f"] + .iter() + .map(|k| (k.to_string(), InputTransform::Static { value: to_raw_value(&1) })) + .collect(); + + let filtered = filter_schema_by_input_transforms(schema, &preset).unwrap(); + let value: serde_json::Value = serde_json::from_str(filtered.get()).unwrap(); + let remaining: Vec<&String> = value["properties"].as_object().unwrap().keys().collect(); + assert_eq!(remaining, ["b", "d", "e", "g", "h"]); + } +}