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,