From fc541c12efd33f6499102021d5de4c0036ee09e0 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Fri, 18 Sep 2026 11:52:55 +0200 Subject: [PATCH] Offer "Use default" only where a field declares one The control was derived from the field being optional, which put it on nearly every field in every dialog. It now comes from the schema's x-ui-defaults-when-empty, which the server sets per field: "you may leave this blank" and "leaving this blank means something specific" are different claims, and only the second is worth a control. Two rules still sit above the declaration. A field the current state requires offers nothing - clearing it would go towards a default that does not exist - and a checkbox is excluded, because false is a value, not an absence. Sampling parameters a provider does not apply are no longer offered either. All five were shown to every provider and the unsupported ones were dropped at run time, reported in a warning on an execution that had already happened. The filter is shared by the two editing surfaces so they cannot drift, and it keeps every knob whenever the answer is not knowable - no provider chosen, one the server does not list, a capability call that failed - because a knob wrongly hidden cannot be set at all, which is worse than one that does nothing. Co-Authored-By: Claude Opus 5 (1M context) --- src/app/models/llm-provider.ts | 10 +++ .../model-parameter-support.spec.ts | 56 +++++++++++++ .../llm-provider/model-parameter-support.ts | 52 ++++++++++++ .../llm-descriptor-settings.ts | 46 ++++++++--- .../container-node/container-node.spec.ts | 12 ++- .../nodes/container-node/container-node.ts | 3 +- .../nodes/generic-node/generic-node.spec.ts | 81 +++++++++++++++++-- .../shared/nodes/generic-node/generic-node.ts | 50 +++++++++++- .../shared/nodes/schema-driven-fields.spec.ts | 27 ++++++- src/app/shared/nodes/schema-driven-fields.ts | 14 +++- .../nodes/task-step-node/task-step-node.ts | 1 + 11 files changed, 325 insertions(+), 27 deletions(-) create mode 100644 src/app/services/llm-provider/model-parameter-support.spec.ts create mode 100644 src/app/services/llm-provider/model-parameter-support.ts diff --git a/src/app/models/llm-provider.ts b/src/app/models/llm-provider.ts index a7b7e8a..77c08e4 100644 --- a/src/app/models/llm-provider.ts +++ b/src/app/models/llm-provider.ts @@ -7,6 +7,16 @@ export type LlmProviderCapability = { requiresCredential: boolean; /** Whether picking this provider means the saved credential must also carry a base URL. */ requiresEndpoint: boolean; + /** Whether this provider can be given tools to call, so a node may bind an MCP server to it. */ + supportsTools?: boolean; + /** + * Which sampling parameters this provider actually applies, by their server-side enum name + * (TEMPERATURE, TOP_P, TOP_K, MAX_TOKENS, SEED). Every provider is offered the same five, and one + * it does not apply is dropped at run time with a warning - which is too late to be useful, so the + * editor uses this to stop offering it. Optional: an older server does not send it, and the + * conservative reading of a missing list is "no claim", not "supports nothing". + */ + supportedParameters?: string[]; }; export type ExecutionVaultCredential = { diff --git a/src/app/services/llm-provider/model-parameter-support.spec.ts b/src/app/services/llm-provider/model-parameter-support.spec.ts new file mode 100644 index 0000000..8126cb4 --- /dev/null +++ b/src/app/services/llm-provider/model-parameter-support.spec.ts @@ -0,0 +1,56 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +import { describe, expect, it } from 'vitest'; +import { LlmProviderCapability } from '@models/llm-provider'; +import { findCapability, isModelParameterField, providerAppliesParameter } from './model-parameter-support'; + +const gemini: LlmProviderCapability = { + name: 'Gemini', + requiresCredential: true, + requiresEndpoint: false, + supportedParameters: ['MAX_TOKENS', 'TEMPERATURE', 'TOP_K', 'TOP_P'] +}; + +describe('providerAppliesParameter', () => { + it('drops only the knobs the provider says it ignores', () => { + // Gemini has no seed, so setting one there did nothing and said so only after the run. + expect(providerAppliesParameter(gemini, 'seed')).toBe(false); + expect(providerAppliesParameter(gemini, 'temperature')).toBe(true); + expect(providerAppliesParameter(gemini, 'topK')).toBe(true); + }); + + it('keeps anything that is not a sampling parameter', () => { + // The filter runs over a whole dialog, and must not touch the fields it knows nothing about. + expect(providerAppliesParameter(gemini, 'model')).toBe(true); + expect(providerAppliesParameter(gemini, 'provider')).toBe(true); + }); + + it('offers everything when the answer is not knowable', () => { + // No provider chosen, one the server does not list, or a server too old to report the field: a + // knob wrongly hidden cannot be set at all, which is worse than one that does nothing. + expect(providerAppliesParameter(undefined, 'seed')).toBe(true); + expect(providerAppliesParameter(null, 'seed')).toBe(true); + expect(providerAppliesParameter( + { name: 'Old', requiresCredential: false, requiresEndpoint: false }, 'seed')).toBe(true); + }); + + it('knows which field names are sampling parameters', () => { + expect(isModelParameterField('maxTokens')).toBe(true); + expect(isModelParameterField('topP')).toBe(true); + expect(isModelParameterField('prompt')).toBe(false); + }); +}); + +describe('findCapability', () => { + it('matches the provider name regardless of case and padding', () => { + expect(findCapability([gemini], ' gemini ')?.name).toBe('Gemini'); + }); + + it('answers nothing for an unknown or empty provider', () => { + expect(findCapability([gemini], 'Nope')).toBeUndefined(); + expect(findCapability([gemini], '')).toBeUndefined(); + expect(findCapability(null, 'Gemini')).toBeUndefined(); + }); +}); diff --git a/src/app/services/llm-provider/model-parameter-support.ts b/src/app/services/llm-provider/model-parameter-support.ts new file mode 100644 index 0000000..51fa30b --- /dev/null +++ b/src/app/services/llm-provider/model-parameter-support.ts @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +import { LlmProviderCapability } from '@models/llm-provider'; + +/** + * The bridge between a sampling parameter's field name and the server's own name for it. + * + *

Two spellings for the same five knobs: the editor works in the JSON field names ModelParameters + * serialises to, the capability list reports the ModelParameter enum. Kept in one place so the two + * editing surfaces that filter on it cannot drift apart. + */ +const PARAMETER_ENUM_BY_FIELD: Readonly> = { + temperature: 'TEMPERATURE', + topP: 'TOP_P', + topK: 'TOP_K', + maxTokens: 'MAX_TOKENS', + seed: 'SEED' +}; + +export function isModelParameterField(fieldKey: string): boolean { + return fieldKey in PARAMETER_ENUM_BY_FIELD; +} + +/** + * Whether the chosen provider applies this parameter, so the editor knows not to offer it. + * + *

Answers true for anything that is not one of the five, and true whenever the capability is + * unknown - no provider chosen yet, the provider not in the list, an older server that does not + * report the field. Hiding a control on a guess would be worse than the problem: the run still + * honours whatever is set, and a parameter wrongly hidden is one nobody can set at all. + */ +export function providerAppliesParameter( + capability: LlmProviderCapability | undefined | null, + fieldKey: string +): boolean { + const enumName = PARAMETER_ENUM_BY_FIELD[fieldKey]; + if (!enumName) return true; + const supported = capability?.supportedParameters; + if (!Array.isArray(supported)) return true; + return supported.includes(enumName); +} + +export function findCapability( + capabilities: readonly LlmProviderCapability[] | null | undefined, + provider: string | null | undefined +): LlmProviderCapability | undefined { + const wanted = (provider ?? '').trim().toLowerCase(); + if (!wanted) return undefined; + return (capabilities ?? []).find((capability) => capability.name.trim().toLowerCase() === wanted); +} diff --git a/src/app/shared/llm-descriptor-settings/llm-descriptor-settings.ts b/src/app/shared/llm-descriptor-settings/llm-descriptor-settings.ts index 3edf47a..1079887 100644 --- a/src/app/shared/llm-descriptor-settings/llm-descriptor-settings.ts +++ b/src/app/shared/llm-descriptor-settings/llm-descriptor-settings.ts @@ -9,6 +9,8 @@ import { LlmProviderService } from '@services/llm-provider/llm-provider'; import { FieldRetriever } from '@services/retriever/field-retriever'; import { NodeSettingField, NodeSettingsDialogService } from '@services/dialogs/node-settings-dialog'; import { readSimulatorParameters } from '@shared/task-execution-viewer/execution-viewer.utils'; +import { findCapability, providerAppliesParameter } from '@services/llm-provider/model-parameter-support'; +import { LlmProviderCapability } from '@models/llm-provider'; /** The retrievers the editor already uses for a node's provider and model fields. */ const PROVIDER_RETRIEVER_URL = '/retriever/LLM/providers'; @@ -88,7 +90,13 @@ export type LLMDescriptorSettingsRequest = { export async function openLLMDescriptorSettings( settingsDialog: NodeSettingsDialogService, fieldRetriever: FieldRetriever, - request: LLMDescriptorSettingsRequest + request: LLMDescriptorSettingsRequest, + /** + * Optional, and only used to drop the sampling knobs the chosen provider ignores. A caller that + * does not have it gets the dialog as it was: all five offered, which is the same answer this + * makes when the capability list cannot be read. + */ + llmProviderService?: LlmProviderService ): Promise { const providerOptions = await loadOptions(fieldRetriever, 'providers', {}, PROVIDER_RETRIEVER_URL); if (!providerOptions.length) { @@ -117,17 +125,27 @@ export async function openLLMDescriptorSettings( .map(([key, value]) => [key, String(value)]) ); + // Fetched once and reused: the same list says which parameters a provider applies and whether it + // needs a credential. A failure leaves it empty, which every reader treats as "no claim". + const capabilities = llmProviderService ? await listCapabilitiesQuietly(llmProviderService) : []; + const buildFields = ( providers: { label: string; value: string; }[], - models: { label: string; value: string; }[] + models: { label: string; value: string; }[], + provider: string ): NodeSettingField[] => [ { key: 'provider', label: 'Provider', type: 'select', options: providers, required: true, autofocus: true }, { key: 'model', label: 'Model', type: 'select', options: models, required: true }, - // Inherited sampling is opened, not hidden: a seed carried over from the run being repeated is - // the reason the two runs are comparable, and behind a closed section nobody would see it. - ...PARAMETER_FIELDS.map((field) => (inheritedParameters[field.key] === undefined - ? field - : { ...field, group: undefined })) + // Only the knobs this provider actually applies. The others were accepted here and silently + // dropped at run time, which the execution reported afterwards in a warning nobody was waiting + // for - a box that does nothing is worse than no box. + ...PARAMETER_FIELDS + .filter((field) => providerAppliesParameter(findCapability(capabilities, provider), field.key)) + // Inherited sampling is opened, not hidden: a seed carried over from the run being repeated is + // the reason the two runs are comparable, and behind a closed section nobody would see it. + .map((field) => (inheritedParameters[field.key] === undefined + ? field + : { ...field, group: undefined })) ]; const inheritedModel = inheritedProvider && inherited?.model @@ -137,7 +155,7 @@ export async function openLLMDescriptorSettings( const result = await settingsDialog.open({ title: request.title, - fields: buildFields(providerOptions, initialModelOptions), + fields: buildFields(providerOptions, initialModelOptions, defaultProvider), initial: { provider: defaultProvider, model: inheritedModel ?? initialModelOptions[0]?.value ?? '', @@ -150,7 +168,7 @@ export async function openLLMDescriptorSettings( : []; return { - fields: buildFields(providerOptions, modelOptions), + fields: buildFields(providerOptions, modelOptions, provider), initial: { provider, model: modelOptions[0]?.value ?? '' @@ -211,7 +229,7 @@ export async function openLLMDescriptorSettingsWithCredential( executionVaultCredentials: ExecutionVaultCredentialsService, request: LLMDescriptorSettingsRequest ): Promise { - const descriptor = await openLLMDescriptorSettings(settingsDialog, fieldRetriever, request); + const descriptor = await openLLMDescriptorSettings(settingsDialog, fieldRetriever, request, llmProviderService); if (!descriptor) return null; const needsCredential = await providerRequiresCredential(llmProviderService, descriptor.provider); @@ -221,6 +239,14 @@ export async function openLLMDescriptorSettingsWithCredential( return credentialId ? { descriptor, credentialId } : null; } +async function listCapabilitiesQuietly(llmProviderService: LlmProviderService): Promise { + try { + return await firstValueFrom(llmProviderService.listCapabilities()); + } catch { + return []; + } +} + async function providerRequiresCredential(llmProviderService: LlmProviderService, provider: string): Promise { try { const capabilities = await firstValueFrom(llmProviderService.listCapabilities()); diff --git a/src/app/shared/nodes/container-node/container-node.spec.ts b/src/app/shared/nodes/container-node/container-node.spec.ts index 6453645..755539a 100644 --- a/src/app/shared/nodes/container-node/container-node.spec.ts +++ b/src/app/shared/nodes/container-node/container-node.spec.ts @@ -159,9 +159,10 @@ describe('ContainerNodeComponent', () => { required: ['label'], properties: { label: { type: 'string' }, - retries: { type: 'integer', minimum: 1, maximum: 5 }, - threshold: { type: 'number', minimum: 0, maximum: 1 }, - mode: { type: 'string', default: 'STRICT' } + retries: { type: 'integer', minimum: 1, maximum: 5, 'x-ui-defaults-when-empty': true }, + threshold: { type: 'number', minimum: 0, maximum: 1, 'x-ui-defaults-when-empty': true }, + mode: { type: 'string', default: 'STRICT', 'x-ui-defaults-when-empty': true }, + note: { type: 'string' } } }; (component as any).containerSchema = schema; @@ -189,6 +190,11 @@ describe('ContainerNodeComponent', () => { // Required, so empty is not a state it can be left in. await component.openParameterEditor('label'); expect(openSpy.mock.calls.at(-1)?.[0].fields[0].defaultsWhenEmpty).toBe(false); + + // Optional, but claims nothing about being empty - so the container dialog offers nothing + // either, exactly as the block's does. + await component.openParameterEditor('note'); + expect(openSpy.mock.calls.at(-1)?.[0].fields[0].defaultsWhenEmpty).toBe(false); }); describe('an optional group', () => { diff --git a/src/app/shared/nodes/container-node/container-node.ts b/src/app/shared/nodes/container-node/container-node.ts index fa3e9f1..cac9a14 100644 --- a/src/app/shared/nodes/container-node/container-node.ts +++ b/src/app/shared/nodes/container-node/container-node.ts @@ -569,7 +569,8 @@ export class ContainerNodeComponent implements OnDestroy { minLength: dialogFieldType === 'number' ? undefined : definition.ui.minLength, maxLength: dialogFieldType === 'number' ? undefined : definition.ui.maxLength, pattern: dialogFieldType === 'number' ? undefined : definition.ui.pattern, - defaultsWhenEmpty: !required && dialogFieldType !== 'checkbox', + // Declared by the field, never inferred from it being optional - see SchemaFieldUiMeta. + defaultsWhenEmpty: definition.ui.defaultsWhenEmpty && !required && dialogFieldType !== 'checkbox', defaultValue: this.schemaDeclaredDefault(definition.path) ?? undefined, options: retrieverIsFreeText ? undefined : await this.resolveSelectableOptions(definition) }; diff --git a/src/app/shared/nodes/generic-node/generic-node.spec.ts b/src/app/shared/nodes/generic-node/generic-node.spec.ts index f516eb8..ae9db4e 100644 --- a/src/app/shared/nodes/generic-node/generic-node.spec.ts +++ b/src/app/shared/nodes/generic-node/generic-node.spec.ts @@ -7,6 +7,7 @@ import { DEFAULT_NODE_CAPABILITIES } from '@models/flow'; import { BlocksService } from '@services/blocks/blocks'; import { NodeSettingsDialogService } from '@services/dialogs/node-settings-dialog'; import { FieldRetriever } from '@services/retriever/field-retriever'; +import { LlmProviderService } from '@services/llm-provider/llm-provider'; import { EditorStateHolder } from '@stores/flow-editor'; import { vi } from 'vitest'; import { of, throwError } from 'rxjs'; @@ -27,6 +28,15 @@ describe('GenericNodeComponent', () => { open: vi.fn().mockResolvedValue(null) } }, + { + provide: LlmProviderService, + useValue: { + listCapabilities: vi.fn(() => of([ + { name: 'Gemini', requiresCredential: true, requiresEndpoint: false, + supportedParameters: ['MAX_TOKENS', 'TEMPERATURE', 'TOP_K', 'TOP_P'] } + ])) + } + }, { provide: EditorStateHolder, useValue: { @@ -1008,9 +1018,62 @@ describe('GenericNodeComponent', () => { ]); }); - it('marks every optional property as defaulting when empty, whatever its type', async () => { - // Not a parameter-specific rule: any field the schema does not require means "leave it to - // the default" when empty, and the control has to be able to say so. + it('does not offer a knob the chosen provider ignores', async () => { + // Gemini applies no seed. It was accepted here anyway and dropped at run time, which the + // execution reported afterwards in a warning nobody was waiting for. + const component = fixture.componentInstance as any; + component.optionalGroupFieldDefinitions = [{ + path: 'llmDescriptor.parameters', + label: 'Model parameters', + objectSchema: { + type: 'object', + required: [], + properties: { + temperature: { type: 'number', 'x-ui-defaults-when-empty': true }, + seed: { type: 'integer', 'x-ui-defaults-when-empty': true } + } + }, + ui: { structural: false, visibleWhen: [], enabledWhen: [] } + }]; + component.ensureBlockConfiguration()['llmDescriptor'] = { provider: 'Gemini', model: 'm' }; + const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType; + open.mockResolvedValue(null); + + await component.openOptionalGroupEditor('llmDescriptor.parameters'); + + const keys = open.mock.calls.at(-1)?.[0].fields.map((field: any) => field.key); + expect(keys).toContain('temperature'); + expect(keys).not.toContain('seed'); + }); + + it('keeps every knob when the provider is not one the server lists', async () => { + // Hiding on a guess is worse than the problem: a knob wrongly hidden cannot be set at all. + const component = fixture.componentInstance as any; + component.optionalGroupFieldDefinitions = [{ + path: 'llmDescriptor.parameters', + label: 'Model parameters', + objectSchema: { + type: 'object', + required: [], + properties: { + temperature: { type: 'number', 'x-ui-defaults-when-empty': true }, + seed: { type: 'integer', 'x-ui-defaults-when-empty': true } + } + }, + ui: { structural: false, visibleWhen: [], enabledWhen: [] } + }]; + component.ensureBlockConfiguration()['llmDescriptor'] = { provider: 'Unlisted', model: 'm' }; + const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType; + open.mockResolvedValue(null); + + await component.openOptionalGroupEditor('llmDescriptor.parameters'); + + expect(open.mock.calls.at(-1)?.[0].fields.map((field: any) => field.key)).toContain('seed'); + }); + + it('offers a default only on the properties that declare one', async () => { + // Being optional is not a claim that empty means something: the schema has to say so with + // x-ui-defaults-when-empty, or the control would appear on nearly every field in the editor. const component = fixture.componentInstance as any; component.optionalGroupFieldDefinitions = [{ path: 'llmDescriptor.parameters', @@ -1021,9 +1084,9 @@ describe('GenericNodeComponent', () => { properties: { mode: { type: 'string' }, note: { type: 'string' }, - temperature: { type: 'number' }, - verbose: { type: 'boolean' }, - outcome: { type: 'string', default: 'DONE' } + temperature: { type: 'number', 'x-ui-defaults-when-empty': true }, + verbose: { type: 'boolean', 'x-ui-defaults-when-empty': true }, + outcome: { type: 'string', default: 'DONE', 'x-ui-defaults-when-empty': true } } }, ui: { structural: false, visibleWhen: [], enabledWhen: [] } @@ -1037,10 +1100,12 @@ describe('GenericNodeComponent', () => { const byKey = new Map( open.mock.calls.at(-1)?.[0].fields.map((field: any) => [field.key, field]) ); + // Required, so there is no empty state to return to. expect(byKey.get('mode').defaultsWhenEmpty).toBe(false); - expect(byKey.get('note').defaultsWhenEmpty).toBe(true); + // Optional but says nothing: blank is just blank. + expect(byKey.get('note').defaultsWhenEmpty).toBe(false); expect(byKey.get('temperature').defaultsWhenEmpty).toBe(true); - // A checkbox has no empty state: false is a value. + // A checkbox has no empty state - false is a value - so it is excluded even when it declares one. expect(byKey.get('verbose').defaultsWhenEmpty).toBe(false); expect(byKey.get('outcome')).toMatchObject({ defaultsWhenEmpty: true, defaultValue: 'DONE' }); }); diff --git a/src/app/shared/nodes/generic-node/generic-node.ts b/src/app/shared/nodes/generic-node/generic-node.ts index 416ca1e..aafdd9c 100644 --- a/src/app/shared/nodes/generic-node/generic-node.ts +++ b/src/app/shared/nodes/generic-node/generic-node.ts @@ -22,6 +22,9 @@ import { import { EditorStateHolder } from '@stores/flow-editor'; import { FieldRetriever } from '@services/retriever/field-retriever'; import { BlocksService } from '@services/blocks/blocks'; +import { LlmProviderService } from '@services/llm-provider/llm-provider'; +import { LlmProviderCapability } from '@models/llm-provider'; +import { findCapability, isModelParameterField, providerAppliesParameter } from '@services/llm-provider/model-parameter-support'; import { firstValueFrom, take } from 'rxjs'; import { SWIMLANES_ENABLED } from '@shared/feature-flags'; import { ConditionalRequiredField, extractSchemaRequirements, SchemaRequirements } from '../schema-requirements'; @@ -137,6 +140,7 @@ export class GenericNodeComponent implements OnDestroy { private editorState = inject(EditorStateHolder); private fieldRetriever = inject(FieldRetriever); private blocksService = inject(BlocksService); + private llmProviders = inject(LlmProviderService); private cdr = inject(ChangeDetectorRef); private hostElement = inject>(ElementRef); private readonly focusModal = new NodeFocusModalController(this.hostElement, 'generic-node-focus-placeholder'); @@ -1466,7 +1470,10 @@ export class GenericNodeComponent implements OnDestroy { const dialog = await this.buildObjectDialog(definition.objectSchema, definition.label, currentValue); if (!dialog) return; - const result = await this.settingsDialog.open(dialog); + const result = await this.settingsDialog.open({ + ...dialog, + fields: await this.dropParametersTheProviderIgnores(path, dialog.fields) + }); if (!result) return; const next = this.parseObjectDialogResult(definition.objectSchema, result, currentValue); @@ -1577,6 +1584,47 @@ export class GenericNodeComponent implements OnDestroy { }; } + /** + * Drops the sampling knobs the chosen provider does not apply. + * + *

The provider sits beside the parameters it samples with - `llmDescriptor.provider` next to + * `llmDescriptor.parameters` - which is what makes this answerable while editing. Until now every + * provider was offered all five and the ones it ignores were dropped at run time, reported in a + * warning on an execution that had already happened. + * + *

Leaves everything in place when the answer is not knowable: no provider chosen yet, a + * provider the server does not list, or a capability call that failed. A knob wrongly hidden + * cannot be set at all, which is worse than one that does nothing. + */ + private async dropParametersTheProviderIgnores( + path: string, + fields: NodeSettingField[] + ): Promise { + if (!fields.some((field) => isModelParameterField(field.key))) return fields; + + const parentPath = path.split('.').slice(0, -1).join('.'); + const providerPath = parentPath ? `${parentPath}.provider` : 'provider'; + const provider = this.getByPath(this.ensureBlockConfiguration(), providerPath); + if (typeof provider !== 'string' || !provider.trim()) return fields; + + const capability = findCapability(await this.llmProviderCapabilities(), provider); + if (!capability) return fields; + return fields.filter((field) => providerAppliesParameter(capability, field.key)); + } + + /** Fetched at most once per node, and never fatal: an empty list means "no claim". */ + private async llmProviderCapabilities(): Promise { + if (this.cachedLlmCapabilities) return this.cachedLlmCapabilities; + try { + this.cachedLlmCapabilities = await firstValueFrom(this.llmProviders.listCapabilities()); + } catch { + this.cachedLlmCapabilities = []; + } + return this.cachedLlmCapabilities; + } + + private cachedLlmCapabilities: LlmProviderCapability[] | null = null; + private buildObjectDialog( objectSchema: Record | null, title: string, diff --git a/src/app/shared/nodes/schema-driven-fields.spec.ts b/src/app/shared/nodes/schema-driven-fields.spec.ts index 9230aa7..b4ec849 100644 --- a/src/app/shared/nodes/schema-driven-fields.spec.ts +++ b/src/app/shared/nodes/schema-driven-fields.spec.ts @@ -736,10 +736,14 @@ describe('buildTemplatedRichContentParts', () => { type: 'object', required: [], properties: { - sourceType: { type: 'string', enum: ['CATALOG', 'CUSTOM'], default: 'CATALOG', title: 'Source type' }, + sourceType: { + type: 'string', enum: ['CATALOG', 'CUSTOM'], default: 'CATALOG', title: 'Source type', + 'x-ui-defaults-when-empty': true + }, serverName: { type: 'string', title: 'Server name', + 'x-ui-defaults-when-empty': true, // The key the producer emits for @UiRequiredWhen(equalsAny = ...) is `in`, not `equalsAny`. 'x-ui-required-when': { field: 'sourceType', in: ['CATALOG', ''] } } @@ -777,18 +781,37 @@ describe('buildTemplatedRichContentParts', () => { const custom = await buildSchemaObjectDialog(mcpBindingSchema, 'MCP server', { sourceType: 'CUSTOM' }, dialogHooks); // "Use default" cleared serverName straight into an invalid state, towards a default that does - // not exist. It belongs only where emptying the field is a legitimate answer. + // not exist. Being required by the current state overrules the field's own declaration: the + // schema says empty is meaningful, the state says not right now, and the state wins. expect(catalog.fields.find((field) => field.key === 'serverName')!.defaultsWhenEmpty).toBe(false); expect(custom.fields.find((field) => field.key === 'serverName')!.defaultsWhenEmpty).toBe(true); }); it('names the default an empty field is using, when the schema declares one', async () => { + // The declaration turns the control on; JSON Schema's own `default` supplies the value to name. + // They are separate on purpose: most fields carrying a `default` are describing what the backend + // assumes, not inviting the editor to offer a way back to it. const dialog = await buildSchemaObjectDialog(mcpBindingSchema, 'MCP server', {}, dialogHooks); const sourceType = dialog.fields.find((field) => field.key === 'sourceType')!; expect(sourceType.defaultsWhenEmpty).toBe(true); expect(sourceType.defaultValue).toBe('CATALOG'); }); + + it('leaves an optional field alone when it declares no default', async () => { + const schema = { + type: 'object', + required: [], + properties: { note: { type: 'string', title: 'Note' } } + }; + + const dialog = await buildSchemaObjectDialog(schema, 'Notes', {}, { + schemaRoot: schema, + loadOptions: () => [] + }); + + expect(dialog.fields.find((field) => field.key === 'note')!.defaultsWhenEmpty).toBe(false); + }); }); describe('buildSchemaObjectDialog - free text on a retriever-backed property', () => { diff --git a/src/app/shared/nodes/schema-driven-fields.ts b/src/app/shared/nodes/schema-driven-fields.ts index 459e48b..0311e7f 100644 --- a/src/app/shared/nodes/schema-driven-fields.ts +++ b/src/app/shared/nodes/schema-driven-fields.ts @@ -36,6 +36,13 @@ export type SchemaRetrieverFieldDefinition = { export type SchemaFieldUiMeta = { widget: 'textarea' | null; acceptVariableAsPlaceholder: boolean; + /** + * Whether leaving this field empty is a decision the editor should let you go back to, as the + * backend's @DefaultsWhenEmpty declares it. Not inferred from the field being optional: most + * optional fields are simply blank, and offering a way back to blank on all of them put the + * control almost everywhere it was not wanted. + */ + defaultsWhenEmpty: boolean; structural: boolean; bindableAsInput: boolean; inputName: string | null; @@ -151,6 +158,7 @@ export function toSchemaFieldUiMeta( ? Math.trunc(rowsRaw) : undefined; const acceptVariableAsPlaceholder = schema?.['x-ui-accept-variable-as-placeholder'] === true; + const defaultsWhenEmpty = schema?.['x-ui-defaults-when-empty'] === true; const structural = schema?.['x-ui-structural'] === true; const bindableAsInput = schema?.['x-ui-bindable-as-input'] === true; const inputName = typeof schema?.['x-ui-input-name'] === 'string' @@ -184,6 +192,7 @@ export function toSchemaFieldUiMeta( return { widget: normalizedWidget, acceptVariableAsPlaceholder, + defaultsWhenEmpty, structural, bindableAsInput, inputName, @@ -1017,8 +1026,9 @@ export async function buildSchemaObjectDialog( * serverName was being saved as valid. */ required: requiredByState, - // A checkbox is excluded because it has no empty state: false is a value, not an absence. - defaultsWhenEmpty: !requiredNow && fieldType !== 'checkbox', + // Declared by the field, never inferred from it being optional. A checkbox is still excluded + // even when it declares one, because it has no empty state: false is a value, not an absence. + defaultsWhenEmpty: fieldUi.defaultsWhenEmpty && !requiredNow && fieldType !== 'checkbox', defaultValue: propertySchema?.['default'] == null ? undefined : String(propertySchema['default']), readonly: !fieldUi.enabledWhen.every((rule) => evaluateUiConditionRule(rule, value, (fieldPath) => resolveSchemaPath(objectSchema, fieldPath)) diff --git a/src/app/shared/nodes/task-step-node/task-step-node.ts b/src/app/shared/nodes/task-step-node/task-step-node.ts index 28bebe5..78c9402 100644 --- a/src/app/shared/nodes/task-step-node/task-step-node.ts +++ b/src/app/shared/nodes/task-step-node/task-step-node.ts @@ -1260,6 +1260,7 @@ export class TaskStepNodeComponent { ui: { widget: null, acceptVariableAsPlaceholder: false, + defaultsWhenEmpty: false, structural: true, bindableAsInput: false, inputName: null,