mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: keep a legacy memory count unset on open and read a cleared count as off
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
24feb11cef
commit
74bcf90ca0
@@ -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<Uuid>,
|
||||
@@ -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<usize, D::Error> {
|
||||
<Option<usize> as serde::Deserialize>::deserialize(deserializer).map(Option::unwrap_or_default)
|
||||
}
|
||||
|
||||
fn deserialize_present<'de, D: serde::Deserializer<'de>>(
|
||||
deserializer: D,
|
||||
) -> Result<Option<serde_json::Value>, D::Error> {
|
||||
|
||||
@@ -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 } }),
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 } }
|
||||
})
|
||||
|
||||
@@ -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<string, any> = {
|
||||
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<string, any> = {
|
||||
/** 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 }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user