From 74bcf90ca004db9a1558f6ebf33878afeaf422e7 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 16 Sep 2026 11:27:23 +0200 Subject: [PATCH] fix: keep a legacy memory count unset on open and read a cleared count as off Co-Authored-By: Claude Fable 5.1 --- backend/windmill-ai/src/types.rs | 11 ++++++++++- backend/windmill-worker/src/ai_executor.rs | 6 ++++++ frontend/src/lib/components/ModuleTest.svelte | 9 +++++++-- .../lib/components/flows/agentFormFields.test.ts | 8 ++++++++ .../flows/content/AiAgentStepInputs.svelte | 2 +- frontend/src/lib/components/flows/flowInfers.ts | 13 ++++++++----- 6 files changed, 40 insertions(+), 9 deletions(-) diff --git a/backend/windmill-ai/src/types.rs b/backend/windmill-ai/src/types.rs index c1ff30d9a5..9de3ca38cf 100644 --- a/backend/windmill-ai/src/types.rs +++ b/backend/windmill-ai/src/types.rs @@ -79,11 +79,12 @@ impl Default for OutputType { pub enum Memory { Off, Window { + #[serde(default, deserialize_with = "deserialize_null_as_zero")] context_length: usize, }, /// Written before `window`. Its `memory_id` stays a fallback behind the run's memory id. Auto { - #[serde(default)] + #[serde(default, deserialize_with = "deserialize_null_as_zero")] context_length: usize, #[serde(default, deserialize_with = "deserialize_blank_as_none")] memory_id: Option, @@ -107,6 +108,14 @@ fn deserialize_blank_as_none<'de, D: serde::Deserializer<'de>>( } } +// A count the editor's number field was cleared of is stored as `null`, which `default` does not +// cover; it reads as 0, memory off, rather than failing every run of the step. +fn deserialize_null_as_zero<'de, D: serde::Deserializer<'de>>( + deserializer: D, +) -> Result { + as serde::Deserialize>::deserialize(deserializer).map(Option::unwrap_or_default) +} + fn deserialize_present<'de, D: serde::Deserializer<'de>>( deserializer: D, ) -> Result, D::Error> { diff --git a/backend/windmill-worker/src/ai_executor.rs b/backend/windmill-worker/src/ai_executor.rs index a00cbaef07..c36f78ca5c 100644 --- a/backend/windmill-worker/src/ai_executor.rs +++ b/backend/windmill-worker/src/ai_executor.rs @@ -2053,6 +2053,12 @@ mod tests { Some(run), Resolved::Stateless { noted: false }, ), + ( + "a cleared count is off", + json!({ "memory": { "kind": "window", "context_length": null } }), + Some(run), + Resolved::Stateless { noted: false }, + ), ( "legacy manual replays its messages", json!({ "memory": { "kind": "manual", "messages": message } }), diff --git a/frontend/src/lib/components/ModuleTest.svelte b/frontend/src/lib/components/ModuleTest.svelte index 0776a5a43a..cf13703e33 100644 --- a/frontend/src/lib/components/ModuleTest.svelte +++ b/frontend/src/lib/components/ModuleTest.svelte @@ -176,11 +176,16 @@ // ones (`flowLocalAgentSchema`). Overlaying those would shadow the brain the draft just // supplied with nothing, so an inlined step takes only the inputs its form actually offers. // A history input left blank here stays as the step authored it: turned into an expression that - // evaluates to nothing, it would read as a memory id set to empty. + // evaluates to nothing, it would read as a memory id set to empty, where the step's own blank + // static value reads as unset. const formKeys = ( draft ? (AGENT_FLOW_LOCAL_KEYS as readonly string[]) : Object.keys(args) ).filter( - (key) => !(AGENT_HISTORY_KEYS as readonly string[]).includes(key) || args[key] != undefined + (key) => + !(AGENT_HISTORY_KEYS as readonly string[]).includes(key) || + (args[key] != undefined && + args[key] !== '' && + !(Array.isArray(args[key]) && !args[key].length)) ) // The test form only covers the schema it was given, and for a standalone agent that may be diff --git a/frontend/src/lib/components/flows/agentFormFields.test.ts b/frontend/src/lib/components/flows/agentFormFields.test.ts index f21d8d793b..4d885dc874 100644 --- a/frontend/src/lib/components/flows/agentFormFields.test.ts +++ b/frontend/src/lib/components/flows/agentFormFields.test.ts @@ -134,5 +134,13 @@ describe('memoryPropertyFor', () => { expect( autoVariant({ kind: 'auto', context_length: 4, memory_id: 'x' }).properties.memory_id ).toBeDefined() + // A chat flow drops the baked id on save, so the form does not offer it there. + expect( + memoryPropertyFor( + property, + { kind: 'auto', context_length: 4, memory_id: 'x' }, + true + ).oneOf.at(-1).properties.memory_id + ).toBeUndefined() }) }) diff --git a/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte b/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte index 4b3dbb79a1..31abd3c0b5 100644 --- a/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte +++ b/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte @@ -197,7 +197,7 @@ let memoryFieldSchema = $derived.by(() => { const property = schemaProperties.memory const value = args?.memory?.type === 'static' ? args.memory.value : undefined - const withLegacy = memoryPropertyFor(property, value) + const withLegacy = memoryPropertyFor(property, value, chatInputEnabled) if (withLegacy === property) return schema return { ...schema, properties: { ...schemaProperties, memory: withLegacy } } }) diff --git a/frontend/src/lib/components/flows/flowInfers.ts b/frontend/src/lib/components/flows/flowInfers.ts index 43b71123b5..ce8fea5ff4 100644 --- a/frontend/src/lib/components/flows/flowInfers.ts +++ b/frontend/src/lib/components/flows/flowInfers.ts @@ -194,14 +194,15 @@ export const AI_AGENT_SCHEMA: Schema = { } /** Memory shapes older editors wrote. The step form offers one only to a step that still holds it, - * since the one-of field rewrites a value that matches none of its options. */ + * since the one-of field rewrites a value that matches none of its options. No field carries a + * default: the form writes one into a missing field on open, and a missing count runs as off. */ export const LEGACY_MEMORY_VARIANTS: Record = { auto: { type: 'object', title: 'auto', properties: { kind: { type: 'string', enum: ['auto'] }, - context_length: { type: 'number', title: 'Messages to keep', default: 10 }, + context_length: { type: 'number', title: 'Messages to keep' }, memory_id: { type: 'string', title: 'Fixed memory id' } }, required: ['kind'] @@ -220,13 +221,15 @@ export const LEGACY_MEMORY_VARIANTS: Record = { /** The memory property to render for a value: a legacy kind is added as an option only while the * value holds it. Otherwise the property itself is returned, which callers compare by identity to * avoid rebuilding the step schema. */ -export function memoryPropertyFor(property: any, value: any): any { +export function memoryPropertyFor(property: any, value: any, chatInputEnabled = false): any { let legacy = value?.kind ? LEGACY_MEMORY_VARIANTS[value.kind] : undefined if (!legacy || !property?.oneOf) return property // The form fills an empty string field with `''` when it opens, so the baked id field is only // offered to a value saved with the key. Keyed on presence rather than content, or clearing the - // id to retype it would remove the field mid-edit. - if (value.kind === 'auto' && !('memory_id' in value)) { + // id to retype it would remove the field mid-edit. A chat flow runs on the conversation id and + // drops the baked one on save, so the field is not offered there; the nested form then removes + // the key from the value on open, as the hidden field did before. + if (value.kind === 'auto' && (chatInputEnabled || !('memory_id' in value))) { const { memory_id: _, ...properties } = legacy.properties legacy = { ...legacy, properties } }