From a6a811549a352f0f9557431c3f553d857554f64b Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Mon, 7 Sep 2026 11:53:40 +0200 Subject: [PATCH] Enforce schema bounds in the editors, and say what an empty field does Every bound was already in the schema and already enforced by the server, but nothing passed it to the control: a temperature of 5 was typeable and only failed on save. The settings dialog and the inline node editor now share one validator, so a bound declared once reads the same wherever a value can be typed. Numeric properties finally get a numeric control. Arrow increment and required granularity are kept apart: step says what the value must be a multiple of - 1 on an integer, nothing on a decimal - while stepIncrement only moves the spinner. Arrows on a 0-to-1 field used to jump by 1, reaching only the two ends of the range; they now move by a tenth without making 0.35 wrong. FieldValueConstraints omits stepIncrement so the increment cannot reach the validator to try. An empty optional field now states that it is using the default, with a reset beside the control that stays in place and greys out rather than appearing once a value is typed. Going back to unset is the one thing a filled box cannot express: clearing it by hand looks identical to never having decided. Generic - it follows from the schema not requiring the field, on all three editing surfaces, container included. Also fixes the dialog reading as broken: descriptions were rendered twice, once as a mat-hint and once below in error red, and the wrapping hint overflowed the fixed-height subscript area onto the button beside it. An optional group now sits in the fieldset of the object that owns it, so a node holding two LLM descriptors cannot show two identical "Model parameters" controls with nothing to tell them apart. Co-Authored-By: Claude Opus 5 (1M context) --- .../services/dialogs/node-settings-dialog.ts | 91 +++++++ .../retriever/field-retriever-call.base.ts | 11 + .../retriever/field-retriever-call.fake.ts | 21 +- .../retriever/field-retriever-call.ts | 30 ++- src/app/services/retriever/field-retriever.ts | 14 + .../node-settings-dialog.html | 63 ++++- .../node-settings-dialog.spec.ts | 251 +++++++++++++++++- .../node-settings-dialog.ts | 64 ++++- .../container-node/container-node.spec.ts | 41 +++ .../nodes/container-node/container-node.ts | 37 ++- .../nodes/generic-node/generic-node.html | 21 +- .../nodes/generic-node/generic-node.spec.ts | 247 ++++++++++++++++- .../shared/nodes/generic-node/generic-node.ts | 143 +++++++++- .../shared/nodes/schema-driven-fields.spec.ts | 81 +++++- src/app/shared/nodes/schema-driven-fields.ts | 45 +++- src/styles.css | 37 +++ 16 files changed, 1145 insertions(+), 52 deletions(-) diff --git a/src/app/services/dialogs/node-settings-dialog.ts b/src/app/services/dialogs/node-settings-dialog.ts index 6751820..a627983 100644 --- a/src/app/services/dialogs/node-settings-dialog.ts +++ b/src/app/services/dialogs/node-settings-dialog.ts @@ -21,6 +21,31 @@ export type NodeSettingField = { /** Bounds for a `number` field, so the input refuses out-of-range values as you type. */ min?: number; max?: number; + /** + * Granularity the value must respect, checked by {@link validateFieldValue}: 1 on an integer + * field, absent on a decimal, where the schema declares no granularity and 0.35 is as valid as + * 0.3. Deliberately not what the spinner arrows move by - see {@link stepIncrement}. + */ + step?: number; + /** + * How far one press of a spinner arrow moves the value. A presentation detail, never a + * constraint: `type="number"` with no step arrows by 1, which on a 0-to-1 temperature means the + * arrows can only jump between the two ends of the range. + */ + stepIncrement?: number; + /** Length bounds and format for a text field, checked by {@link validateFieldValue}. */ + minLength?: number; + maxLength?: number; + pattern?: string; + /** + * Empty is a meaningful state here: nothing is sent, and whatever default applies takes over. + * Set on every optional field, so an empty box reads as a decision rather than as unfinished + * work - and so there is a way back to it once a value has been typed, which a text box on its + * own cannot express. + */ + defaultsWhenEmpty?: boolean; + /** What that default is, when the schema declares one. */ + defaultValue?: string; /** * Puts the field in a collapsible section of this name, closed until opened. For settings that * are optional and rarely touched, so they stop competing with the ones you came here for. @@ -31,6 +56,72 @@ export type NodeSettingField = { export type NodeSettingsValues = Record; +/** What can make a value wrong, independently of where it is being edited. */ +export type FieldValueConstraints = Pick< + NodeSettingField, + 'type' | 'required' | 'min' | 'max' | 'step' | 'minLength' | 'maxLength' | 'pattern' +>; + +/** + * Why a value cannot be saved, or null when it can. + * + *

Shared between the settings dialog and the inline node editor so a bound declared once is + * enforced everywhere it can be typed. The native input attributes stop most of it, but nothing + * stops a paste, and a browser that refuses to show the value is not the same as one that refuses + * to save it - the server rejecting a temperature of 5 after the fact is the failure this avoids. + * + *

An empty value is only an error when the field is required: everywhere else empty means + * "unset, leave it to the default", which is the whole contract of an optional parameter. + */ +export function validateFieldValue( + constraints: FieldValueConstraints, + value: string | boolean | number | null | undefined +): string | null { + if (constraints.type === 'checkbox' || constraints.type === 'display') return null; + + const text = typeof value === 'string' ? value.trim() : value == null ? '' : String(value); + if (text.length === 0) { + return constraints.required === true ? 'Required' : null; + } + + if (constraints.type === 'number') { + const numeric = Number(text); + if (!Number.isFinite(numeric)) return 'Must be a number'; + if (constraints.min != null && constraints.max != null && (numeric < constraints.min || numeric > constraints.max)) { + return `Must be between ${constraints.min} and ${constraints.max}`; + } + if (constraints.min != null && numeric < constraints.min) return `Must be ${constraints.min} or more`; + if (constraints.max != null && numeric > constraints.max) return `Must be ${constraints.max} or less`; + if (typeof constraints.step === 'number' && constraints.step > 0) { + // Native step semantics: multiples of step counted from the lower bound. A step of 1 is how + // an integer field says so, and 'Must be a whole number' is what that means to read. + const offset = (numeric - (constraints.min ?? 0)) / constraints.step; + if (Math.abs(offset - Math.round(offset)) > 1e-9) { + return constraints.step === 1 ? 'Must be a whole number' : `Must be a multiple of ${constraints.step}`; + } + } + return null; + } + + if (constraints.minLength != null && text.length < constraints.minLength) { + return `At least ${constraints.minLength} characters`; + } + if (constraints.maxLength != null && text.length > constraints.maxLength) { + return `At most ${constraints.maxLength} characters`; + } + if (constraints.pattern) { + // A pattern the schema got wrong must not lock the field: an uncompilable regex is our bug, + // and the server still has the real say. + try { + if (!new RegExp(constraints.pattern).test(text)) return 'Invalid format'; + } catch { + return null; + } + } + + return null; +} + export type NodeSettingsDialogRefresh = { fields: NodeSettingField[]; initial?: NodeSettingsValues; diff --git a/src/app/services/retriever/field-retriever-call.base.ts b/src/app/services/retriever/field-retriever-call.base.ts index 458557c..fa44a88 100644 --- a/src/app/services/retriever/field-retriever-call.base.ts +++ b/src/app/services/retriever/field-retriever-call.base.ts @@ -36,6 +36,17 @@ export abstract class FieldRetrieverCallServiceBase { retrieverUrl?: string | null ): Observable; + /** + * Whether the retrieved values are an incomplete list, so the field takes a typed value too. + * True for a hosted LLM catalogue, which cannot be enumerated without a credential. + */ + abstract isFieldOpen( + blockType: string, + key: string, + context?: Record, + retrieverUrl?: string | null + ): Observable; + abstract retrieveSchema( schemaUrl: string, context?: Record diff --git a/src/app/services/retriever/field-retriever-call.fake.ts b/src/app/services/retriever/field-retriever-call.fake.ts index 4f05a13..f48ef92 100644 --- a/src/app/services/retriever/field-retriever-call.fake.ts +++ b/src/app/services/retriever/field-retriever-call.fake.ts @@ -7,11 +7,15 @@ export class FieldRetrieverCallServiceFake extends FieldRetrieverCallServiceBase }; private readonly modelsByProvider: Record = { - OpenAI: ["gpt-4.1-mini", "gpt-4.1"], - Anthropic: ["claude-3-5-sonnet", "claude-3-7-sonnet"], OllamaTestProvider: ["sam860/gemma3:270m", "llama3.2:3b"] }; + /** + * The hosted providers list nothing, exactly as the real ones do: their catalogues cannot be + * enumerated without a credential, so the editor takes a typed model name instead. + */ + private readonly openModelProviders = new Set(["OpenAI", "Anthropic", "Gemini"]); + private readonly subFlowItems = [ { descriptor: { @@ -140,6 +144,19 @@ export class FieldRetrieverCallServiceFake extends FieldRetrieverCallServiceBase return of(false); } + override isFieldOpen( + _blockType: string, + key: string, + context?: Record, + _retrieverUrl?: string | null + ): Observable { + if (key !== "models") { + return of(false); + } + const provider = context?.["provider"] ?? ""; + return of(this.openModelProviders.has(provider)); + } + override retrieveSchema( schemaUrl: string, context?: Record diff --git a/src/app/services/retriever/field-retriever-call.ts b/src/app/services/retriever/field-retriever-call.ts index c07b0f5..33688a0 100644 --- a/src/app/services/retriever/field-retriever-call.ts +++ b/src/app/services/retriever/field-retriever-call.ts @@ -4,6 +4,9 @@ import { environment } from "@environment"; import { map, Observable, of } from "rxjs"; import { FieldRetrieverCallServiceBase, RetrieverStructuredItem } from "./field-retriever-call.base"; +/** The boolean questions a retriever answers about a field, each on its own sibling endpoint. */ +type RetrieverQuestion = 'required' | 'open'; + export class FieldRetrieverCallService extends FieldRetrieverCallServiceBase { private readonly http = inject(HttpClient); @@ -37,8 +40,19 @@ export class FieldRetrieverCallService extends FieldRetrieverCallServiceBase { context?: Record, retrieverUrl?: string | null ): Observable { - const requiredRetrieverUrl = this.appendRequiredSuffix(retrieverUrl); - const { url, params } = this.resolveRequest(blockType, key, context, requiredRetrieverUrl, true); + const requiredRetrieverUrl = this.appendSuffix(retrieverUrl, 'required'); + const { url, params } = this.resolveRequest(blockType, key, context, requiredRetrieverUrl, 'required'); + return this.http.get(url, { params }); + } + + override isFieldOpen( + blockType: string, + key: string, + context?: Record, + retrieverUrl?: string | null + ): Observable { + const openRetrieverUrl = this.appendSuffix(retrieverUrl, 'open'); + const { url, params } = this.resolveRequest(blockType, key, context, openRetrieverUrl, 'open'); return this.http.get(url, { params }); } @@ -70,9 +84,9 @@ export class FieldRetrieverCallService extends FieldRetrieverCallServiceBase { key: string, context?: Record, retrieverUrl?: string | null, - isRequired = false + suffix?: RetrieverQuestion ) { - const fallbackUrl = `${environment.apiUrl}/retriever/${encodeURIComponent(blockType)}/${encodeURIComponent(key)}${isRequired ? '/required' : ''}`; + const fallbackUrl = `${environment.apiUrl}/retriever/${encodeURIComponent(blockType)}/${encodeURIComponent(key)}${suffix ? `/${suffix}` : ''}`; const baseUrl = this.resolveApiUrl(retrieverUrl) ?? fallbackUrl; const parsed = this.parseUrl(baseUrl); let params = parsed.params; @@ -109,10 +123,14 @@ export class FieldRetrieverCallService extends FieldRetrieverCallServiceBase { return { url, params }; } - private appendRequiredSuffix(rawUrl?: string | null): string | null { + /** + * The yes/no endpoints sit beside the values one, so their URL is the configured retriever URL + * with a suffix - no second URL to declare on the field. + */ + private appendSuffix(rawUrl: string | null | undefined, suffix: RetrieverQuestion): string | null { if (typeof rawUrl !== 'string' || rawUrl.trim().length === 0) return null; const [path, queryString] = rawUrl.split('?', 2); - const normalizedPath = path.endsWith('/required') ? path : `${path}/required`; + const normalizedPath = path.endsWith(`/${suffix}`) ? path : `${path}/${suffix}`; return queryString ? `${normalizedPath}?${queryString}` : normalizedPath; } diff --git a/src/app/services/retriever/field-retriever.ts b/src/app/services/retriever/field-retriever.ts index b7d9c03..b86e9c2 100644 --- a/src/app/services/retriever/field-retriever.ts +++ b/src/app/services/retriever/field-retriever.ts @@ -51,6 +51,20 @@ export class FieldRetriever { ); } + isFieldOpen( + blockType: string, + key: string, + context?: Record, + retrieverUrl?: string | null + ) { + return this.fieldRetrieverCallService.isFieldOpen(blockType, key, context, retrieverUrl).pipe( + catchError((err) => { + console.error('Field open check failed', err); + return throwError(() => err); + }) + ); + } + retrieveSchema( schemaUrl: string, context?: Record diff --git a/src/app/shared/node-settings-dialog/node-settings-dialog.html b/src/app/shared/node-settings-dialog/node-settings-dialog.html index b17157c..0e52792 100644 --- a/src/app/shared/node-settings-dialog/node-settings-dialog.html +++ b/src/app/shared/node-settings-dialog/node-settings-dialog.html @@ -22,7 +22,11 @@ (click)="toggleGroup(group.name, $event)"> {{ group.name }} - @if (!isGroupOpen(group.name) && groupSetCount(group.fields) > 0) { + @if (groupErrorCount(group.fields) > 0) { + + {{ groupErrorCount(group.fields) }} to fix + + } @else if (!isGroupOpen(group.name) && groupSetCount(group.fields) > 0) { {{ groupSetCount(group.fields) }} set @@ -44,7 +48,7 @@ } @else { - + } @@ -54,6 +58,13 @@

+ +
+
@switch (field.type) { @case ('display') {
@@ -75,7 +86,7 @@
} @case ('textarea') { - + {{ field.label }} - } @else if (localEditorHasRetriever) { + } @else if (localEditorHasRetriever && !localEditorFreeText) {