diff --git a/src/app/services/task-executions/task-executions-call.base.ts b/src/app/services/task-executions/task-executions-call.base.ts index 58b6764..3686122 100644 --- a/src/app/services/task-executions/task-executions-call.base.ts +++ b/src/app/services/task-executions/task-executions-call.base.ts @@ -80,6 +80,14 @@ export abstract class TaskExecutionsCallServiceBase { inputName: string, values: string[] ): Observable; + /** + * Sets several global inputs in one request. The single-key endpoint had to be called once per + * input, and those calls raced each other on the same execution. + */ + abstract prepareGlobalInputs( + executionId: string, + values: Record + ): Observable; abstract prepareGlobalFileInput( executionId: string, inputName: string, diff --git a/src/app/services/task-executions/task-executions-call.fake.ts b/src/app/services/task-executions/task-executions-call.fake.ts index bd2dc31..39c8d8d 100644 --- a/src/app/services/task-executions/task-executions-call.fake.ts +++ b/src/app/services/task-executions/task-executions-call.fake.ts @@ -901,6 +901,29 @@ export class TaskExecutionsCallServiceFake extends TaskExecutionsCallServiceBase return of(execution); } + override prepareGlobalInputs( + executionId: string, + values: Record + ): Observable { + const execution = this.findExecution(executionId); + // The viewer reads globals from context.globalInputs and the descriptors, so write them there: + // the older single-key fakes only ever touched context.inputs, where nothing looks for them. + execution.context.globalInputs = { ...(execution.context.globalInputs ?? {}) }; + execution.context.globalInputDescriptors = { ...(execution.context.globalInputDescriptors ?? {}) }; + for (const [inputName, value] of Object.entries(values)) { + execution.context.globalInputs[inputName] = value; + const descriptor = execution.context.globalInputDescriptors[inputName]; + if (descriptor) { + execution.context.globalInputDescriptors[inputName] = { ...descriptor, value }; + } + } + const provided = new Set(Object.keys(values)); + execution.missingGlobalInputKeys = (execution.missingGlobalInputKeys ?? []) + .filter((key) => !provided.has(key)); + execution.context.status = execution.context.waitingSteps.length ? 'WAITING' : execution.context.status; + return of(execution); + } + override prepareGlobalFileInput( executionId: string, inputName: string, diff --git a/src/app/services/task-executions/task-executions-call.ts b/src/app/services/task-executions/task-executions-call.ts index 88b8f0e..dcf2f32 100644 --- a/src/app/services/task-executions/task-executions-call.ts +++ b/src/app/services/task-executions/task-executions-call.ts @@ -256,6 +256,14 @@ export class TaskExecutionsCallService extends TaskExecutionsCallServiceBase { }).pipe(map((raw) => this.mapExecution(raw))); } + override prepareGlobalInputs( + executionId: string, + values: Record + ): Observable { + const url = `${environment.apiUrl}/executions/${encodeURIComponent(executionId)}/globals`; + return this.http.put(url, values).pipe(map((raw) => this.mapExecution(raw))); + } + override prepareGlobalFileInput( executionId: string, inputName: string, diff --git a/src/app/services/task-executions/task-executions.ts b/src/app/services/task-executions/task-executions.ts index 1f40a9b..38e3ad2 100644 --- a/src/app/services/task-executions/task-executions.ts +++ b/src/app/services/task-executions/task-executions.ts @@ -336,6 +336,13 @@ export class TaskExecutionsService { ); } + prepareGlobalInputs(executionId: string, values: Record) { + return this.withRefreshAndErrorHandling( + this.taskExecutionsCallService.prepareGlobalInputs(executionId, values), + 'Prepare global inputs failed' + ); + } + prepareGlobalFileInput(executionId: string, inputName: string, file: File) { return this.withRefreshAndErrorHandling( this.taskExecutionsCallService.prepareGlobalFileInput(executionId, inputName, file), diff --git a/src/app/shared/task-execution-viewer/execution-viewer.utils.spec.ts b/src/app/shared/task-execution-viewer/execution-viewer.utils.spec.ts index b27600b..9febdc9 100644 --- a/src/app/shared/task-execution-viewer/execution-viewer.utils.spec.ts +++ b/src/app/shared/task-execution-viewer/execution-viewer.utils.spec.ts @@ -1,5 +1,14 @@ import { TaskExecution, TaskExecutionStep } from '@models/task-execution'; -import { buildAuthorizationGate, buildVisibleExecutionLogs, getExecutionInputValues, getExecutionOutputValues, hasStoredValue, isExecutionStartable } from './execution-viewer.utils'; +import { + buildAuthorizationGate, + buildVisibleExecutionLogs, + getExecutionInputValues, + getExecutionOutputValues, + hasStoredValue, + isExecutionStartable, + planInputSaves, + preparedInputValue +} from './execution-viewer.utils'; describe('execution viewer runtime values', () => { const documentedStep: TaskExecutionStep = { @@ -188,3 +197,96 @@ describe('hasStoredValue', () => { expect(hasStoredValue(false)).toBe(true); }); }); + +describe('planInputSaves', () => { + function input(overrides: Record = {}): any { + return { + key: 'global:role', scope: 'global', nodeId: null, inputName: 'role', + title: 'Flow', subtitle: 'role', type: 'TEXT', multiple: false, value: '', provided: false, + ...overrides + }; + } + + it('puts every edited global in one map, so one request can carry them all', () => { + // A request per global, fired in parallel, is what lost the values: the bulk endpoint replaced + // the whole set, so whichever save landed last wiped its neighbours. + const inputs = [ + input({ key: 'g:title', inputName: 'positionTitle' }), + input({ key: 'g:req', inputName: 'jobRequirements' }), + input({ key: 'g:questions', inputName: 'interviewQuestions', multiple: true, value: [] }) + ]; + + const plan = planInputSaves(inputs, { + 'g:title': 'Backend Developer', + 'g:req': 'At least 3 years', + 'g:questions': ['first', 'second'] + }); + + expect(plan.globals).toHaveLength(3); + expect(plan.globalValues).toEqual({ + positionTitle: 'Backend Developer', + jobRequirements: 'At least 3 years', + interviewQuestions: ['first', 'second'] + }); + expect(plan.nodeInputs).toEqual([]); + }); + + it('keeps node inputs separate: there is no bulk endpoint per step', () => { + const inputs = [ + input({ key: 'g:role', inputName: 'role' }), + input({ key: 'step-1:cv', scope: 'node', nodeId: 'step-1', inputName: 'cv' }) + ]; + + const plan = planInputSaves(inputs, { 'g:role': 'HR', 'step-1:cv': 'a cv' }); + + expect(Object.keys(plan.globalValues)).toEqual(['role']); + expect(plan.nodeInputs.map((one) => one.key)).toEqual(['step-1:cv']); + }); + + it('ignores anything the user has not edited', () => { + const inputs = [ + input({ key: 'g:role', inputName: 'role', value: 'stored' }), + input({ key: 'g:other', inputName: 'other', value: 'also stored' }) + ]; + + const plan = planInputSaves(inputs, { 'g:role': 'edited' }); + + // Sending an untouched value back would be a write the user did not ask for. + expect(plan.globalValues).toEqual({ role: 'edited' }); + expect(plan.globals).toHaveLength(1); + }); + + it('sends an emptied field, which is an edit like any other', () => { + const plan = planInputSaves([input({ key: 'g:role', inputName: 'role', value: 'was set' })], + { 'g:role': '' }); + + expect(plan.globalValues).toEqual({ role: '' }); + }); +}); + +describe('preparedInputValue', () => { + function listInput(): any { + return { key: 'g:q', scope: 'global', nodeId: null, inputName: 'q', title: 'Flow', + subtitle: 'q', type: 'TEXT', multiple: true, value: [], provided: false }; + } + + it('drops blank items and trims the rest of a list', () => { + expect(preparedInputValue(listInput(), [' first ', '', ' ', 'second'])) + .toEqual(['first', 'second']); + }); + + it('sends a single value as a string, not as a one-item list', () => { + const single: any = { ...listInput(), multiple: false, key: 'g:role', inputName: 'role' }; + expect(preparedInputValue(single, 'Backend Developer')).toBe('Backend Developer'); + }); + + it('takes the first item when a single-valued input somehow holds a list', () => { + const single: any = { ...listInput(), multiple: false }; + expect(preparedInputValue(single, ['first', 'second'])).toBe('first'); + }); + + it('falls back to the stored value when there is no pending edit', () => { + const single: any = { ...listInput(), multiple: false, value: 'stored' }; + expect(preparedInputValue(single, undefined)).toBe('stored'); + }); +}); diff --git a/src/app/shared/task-execution-viewer/execution-viewer.utils.ts b/src/app/shared/task-execution-viewer/execution-viewer.utils.ts index 3b94c19..fff5562 100644 --- a/src/app/shared/task-execution-viewer/execution-viewer.utils.ts +++ b/src/app/shared/task-execution-viewer/execution-viewer.utils.ts @@ -17,6 +17,7 @@ import { TaskExecutionStep, } from '@models/task-execution'; import { LlmProviderCapability } from '@models/llm-provider'; +import { EditableExecutionInput } from '@shared/task-execution-inputs-panel/task-execution-inputs-panel'; export type ExecutionOutputEntry = { key: string; @@ -547,3 +548,48 @@ export function hasStoredValue(value: unknown): boolean { } return String(value).trim().length > 0; } + +/** + * What one Save has to send: every edited global in a single map, and the node inputs one by one. + * + * Splitting it this way is the point. A request per input, fired in parallel, had them all mutating + * the same execution at once - and the bulk global endpoint replaced the whole set, so the list + * input's save arrived last and took the neighbouring typed values with it. + */ +export type PlannedInputSaves = { + globals: EditableExecutionInput[]; + globalValues: Record; + nodeInputs: EditableExecutionInput[]; +}; + +/** The value as the API wants it: a trimmed list of non-empty items, or a single string. */ +export function preparedInputValue( + input: EditableExecutionInput, + pendingValue: string | string[] | undefined +): string | string[] { + const value = pendingValue ?? normalizeEditableInputValue(input.value, input.multiple); + if (input.multiple) { + return (Array.isArray(value) ? value : [String(value)]) + .map((item) => String(item).trim()) + .filter((item) => item.length > 0); + } + return String(Array.isArray(value) ? value[0] ?? '' : value); +} + +export function planInputSaves( + editableInputs: EditableExecutionInput[], + pending: Record +): PlannedInputSaves { + const edited = editableInputs + .filter((input) => Object.prototype.hasOwnProperty.call(pending, input.key)); + const globals = edited.filter((input) => input.scope === 'global'); + const globalValues: Record = {}; + for (const input of globals) { + globalValues[input.inputName] = preparedInputValue(input, pending[input.key]); + } + return { + globals, + globalValues, + nodeInputs: edited.filter((input) => input.scope !== 'global') + }; +} diff --git a/src/app/shared/task-execution-viewer/task-execution-viewer.ts b/src/app/shared/task-execution-viewer/task-execution-viewer.ts index da55f60..1f4d75d 100644 --- a/src/app/shared/task-execution-viewer/task-execution-viewer.ts +++ b/src/app/shared/task-execution-viewer/task-execution-viewer.ts @@ -57,7 +57,7 @@ import { BiasCompareDialogService } from '@services/dialogs/bias-compare-dialog' import { BiasComparisonViewStateService } from '@services/bias/bias-comparison-view-state'; import { BiasImpactReportListComponent } from '@shared/bias-impact-report-list/bias-impact-report-list'; import { JsonViewerComponent } from '@shared/json-viewer/json-viewer'; -import { firstValueFrom, Observable, of, take, tap } from 'rxjs'; +import { catchError, concat, firstValueFrom, Observable, of, take, tap } from 'rxjs'; import { ExecutionOutputEntry, ExecutionOutputGroup, @@ -78,6 +78,8 @@ import { buildExecutionIntermediateInputGroups, buildVisibleExecutionLogs, normalizeEditableInputValue, + planInputSaves, + preparedInputValue, getExecutionInputValues, getExecutionOutputValues, getConnectedInputs, @@ -1119,11 +1121,17 @@ export class TaskExecutionViewerComponent implements OnDestroy { }); } + /** Saves one input, over the same requests the Save bar uses - a global batch of exactly one. */ submitTextInput(input: EditableExecutionInput) { if (this.inputsReadOnly()) return; const executionId = this.execution()?.id; if (!executionId) return; - this.sendPreparedTextInput(input, executionId); + const request$ = input.scope === 'global' + ? this.bulkGlobalRequest([input], + { [input.inputName]: preparedInputValue(input, this.pendingTextInputs()[input.key]) }, + executionId) + : this.singleInputRequest(input, executionId); + request$.subscribe(); } onFileInputChange(input: EditableExecutionInput, files: File[]) { @@ -1244,65 +1252,77 @@ export class TaskExecutionViewerComponent implements OnDestroy { } /** - * Saves every edited input in one go. Each still goes through the same single-input request the - * per-field button used - only the trigger is shared - so a failure is reported per input. + * Saves every edited input. + * + *

The globals go in a single bulk request, and the node inputs follow one at a time. Firing a + * request per input in parallel had them all mutating the same execution at once, and the values + * the user had just typed could come back missing. */ submitAllTextInputs() { if (this.inputsReadOnly()) return; - const pending = new Set(Object.keys(this.pendingTextInputs())); - this.editableInputs() - .filter((input) => pending.has(input.key)) - .forEach((input) => this.submitTextInput(input)); + const executionId = this.execution()?.id; + if (!executionId) return; + + const plan = planInputSaves(this.editableInputs(), this.pendingTextInputs()); + const steps: Observable[] = []; + if (plan.globals.length) { + steps.push(this.bulkGlobalRequest(plan.globals, plan.globalValues, executionId)); + } + for (const input of plan.nodeInputs) { + steps.push(this.singleInputRequest(input, executionId)); + } + if (!steps.length) return; + + // Sequential: one request at a time on one execution, which is the whole point of the change. + concat(...steps).subscribe(); } - private sendPreparedTextInput(input: EditableExecutionInput, executionId: string) { - if (this.inputsReadOnly() || this.execution()?.id !== executionId) return; + /** One request for every edited global, so they cannot overwrite one another. */ + private bulkGlobalRequest( + globals: EditableExecutionInput[], + values: Record, + executionId: string + ): Observable { + globals.forEach((input) => this.setInputSaving(input.key, true)); - const value = this.pendingTextInputs()[input.key] ?? normalizeEditableInputValue(input.value, input.multiple); - this.setInputSaving(input.key, true); - const normalizedValues = (Array.isArray(value) ? value : [String(value)]) - .map((item) => item.trim()) - .filter((item) => item.length > 0); - const request$ = input.scope === 'global' - ? ( - input.multiple - ? this.taskExecutionsService.prepareGlobalStringArrayInput( - executionId, - input.inputName, - normalizedValues - ) - : this.taskExecutionsService.prepareGlobalStringInput( - executionId, - input.inputName, - String(Array.isArray(value) ? value[0] ?? '' : value) - ) - ) - : ( - input.multiple - ? this.taskExecutionsService.prepareStringArrayInput( - executionId, - input.nodeId!, - input.inputName, - normalizedValues - ) - : this.taskExecutionsService.prepareStringInput( - executionId, - input.nodeId!, - input.inputName, - String(Array.isArray(value) ? value[0] ?? '' : value) - ) - ); - - request$.subscribe({ - next: () => { - this.pendingTextInputs.update((current) => { - const next = { ...current }; - delete next[input.key]; - return next; - }); + return this.taskExecutionsService.prepareGlobalInputs(executionId, values).pipe( + tap(() => globals.forEach((input) => { + this.clearPendingInput(input.key); this.clearInputSaving(input.key); - }, - error: () => this.setInputError(input.key, 'Failed to update input') + })), + // The batch failed as a batch, so say so on each input in it rather than guessing a culprit. + catchError(() => { + globals.forEach((input) => this.setInputError(input.key, 'Failed to update inputs')); + return of(null); + }) + ); + } + + /** A node input still goes one at a time: there is no bulk endpoint per step. */ + private singleInputRequest(input: EditableExecutionInput, executionId: string): Observable { + const value = preparedInputValue(input, this.pendingTextInputs()[input.key]); + this.setInputSaving(input.key, true); + const request$ = Array.isArray(value) + ? this.taskExecutionsService.prepareStringArrayInput(executionId, input.nodeId!, input.inputName, value) + : this.taskExecutionsService.prepareStringInput(executionId, input.nodeId!, input.inputName, value); + + return request$.pipe( + tap(() => { + this.clearPendingInput(input.key); + this.clearInputSaving(input.key); + }), + catchError(() => { + this.setInputError(input.key, 'Failed to update input'); + return of(null); + }) + ); + } + + private clearPendingInput(key: string) { + this.pendingTextInputs.update((current) => { + const next = { ...current }; + delete next[key]; + return next; }); }