From 408bcb9309b474db4417a172870d0dfc3da8d187 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Thu, 3 Sep 2026 16:28:26 +0200 Subject: [PATCH] Keep the execution tree's iteration list in step with the run The tree fetched a container's iterations once, when the step was first expanded, and kept that list until the root run id changed. Polling refreshed the run but never that list - and it is the only source both for which children exist and for what state each one is in. So a three-iteration run displayed "Iteration 1, RUNNING" from start to finish. A run that was working looked stuck, and then looked as though it had done one iteration and produced nothing, while the container was in fact on iteration 3 with two results already accumulated. The list now refreshes on each poll tick while the run is live, and once more on the tick that reports it finished - that tick carries the final status and with it the last child's real state. Nothing is fetched for a step nobody opened, a refresh already in flight is not duplicated, and a failed refresh leaves what is on screen alone rather than replacing valid iterations with an error. 493 frontend tests green. Both new assertions fail with the refresh disabled. Co-Authored-By: Claude Opus 5 (1M context) --- .../execution-tree/execution-tree.spec.ts | 83 +++++++++++++++++++ .../shared/execution-tree/execution-tree.ts | 53 +++++++++++- 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/src/app/shared/execution-tree/execution-tree.spec.ts b/src/app/shared/execution-tree/execution-tree.spec.ts index 8a74a9d..a54ca27 100644 --- a/src/app/shared/execution-tree/execution-tree.spec.ts +++ b/src/app/shared/execution-tree/execution-tree.spec.ts @@ -126,6 +126,89 @@ describe('ExecutionTreeComponent', () => { expect(retrieveStepIterations).toHaveBeenCalledTimes(1); }); + it('picks up new iterations and changed statuses on a poll tick', () => { + // The list endpoint is the only source both for which children exist and for what each is + // doing. Fetched once on expand, the tree showed a three-iteration run as "Iteration 1, + // RUNNING" for its whole life: a working run looked stuck, then looked like it had run once. + const running = { ...iteration('it-1', 1, 'MAIN') }; + running.context = { ...running.context, status: 'RUNNING' }; + retrieveStepIterations.mockReturnValue(of([running])); + + const steps = { 'container-1': containerStep({ status: 'WAITING_FOR_SUBFLOW' }) }; + fixture.componentRef.setInput('rootExecution', rootExecution(steps)); + fixture.detectChanges(); + component.toggleStep('root-1', 'container-1'); + fixture.detectChanges(); + + expect(component.iterationsFor('root-1', 'container-1').map((one) => one.context.status)) + .toEqual(['RUNNING']); + + retrieveStepIterations.mockReturnValue(of([ + iteration('it-1', 1, 'MAIN'), + iteration('it-2', 2, 'MAIN') + ])); + // Polling hands the component a new object with the same id, which is the refresh signal. + fixture.componentRef.setInput('rootExecution', rootExecution(steps)); + fixture.detectChanges(); + + expect(component.iterationsFor('root-1', 'container-1').map((one) => one.id)) + .toEqual(['it-1', 'it-2']); + expect(component.iterationsFor('root-1', 'container-1').map((one) => one.context.status)) + .toEqual(['SUCCESS', 'SUCCESS']); + }); + + it('refreshes once more on the tick that reports the run finished', () => { + // That tick carries the final status, and with it the last child's real state. + retrieveStepIterations.mockReturnValue(of([iteration('it-1', 1, 'MAIN')])); + const steps = { 'container-1': containerStep() }; + fixture.componentRef.setInput('rootExecution', rootExecution(steps)); + fixture.detectChanges(); + component.toggleStep('root-1', 'container-1'); + fixture.detectChanges(); + const afterExpand = retrieveStepIterations.mock.calls.length; + + const finished = rootExecution(steps); + finished.context.status = 'SUCCESS'; + fixture.componentRef.setInput('rootExecution', finished); + fixture.detectChanges(); + expect(retrieveStepIterations.mock.calls.length).toBe(afterExpand + 1); + + // And then stops: nothing more will change, so further ticks cost nothing. + const settled = rootExecution(steps); + settled.context.status = 'SUCCESS'; + fixture.componentRef.setInput('rootExecution', settled); + fixture.detectChanges(); + expect(retrieveStepIterations.mock.calls.length).toBe(afterExpand + 1); + }); + + it('refreshes nothing while no step has been opened', () => { + retrieveStepIterations.mockReturnValue(of([iteration('it-1', 1, 'MAIN')])); + const steps = { 'container-1': containerStep() }; + fixture.componentRef.setInput('rootExecution', rootExecution(steps)); + fixture.detectChanges(); + fixture.componentRef.setInput('rootExecution', rootExecution(steps)); + fixture.detectChanges(); + + expect(retrieveStepIterations).not.toHaveBeenCalled(); + }); + + it('keeps the iterations on screen when a refresh fails', () => { + retrieveStepIterations.mockReturnValue(of([iteration('it-1', 1, 'MAIN')])); + const steps = { 'container-1': containerStep() }; + fixture.componentRef.setInput('rootExecution', rootExecution(steps)); + fixture.detectChanges(); + component.toggleStep('root-1', 'container-1'); + fixture.detectChanges(); + + retrieveStepIterations.mockReturnValue(throwError(() => ({ status: 500 }))); + fixture.componentRef.setInput('rootExecution', rootExecution(steps)); + fixture.detectChanges(); + + // Replacing a list with an error would hide iterations that are still perfectly valid. + expect(component.iterationsFor('root-1', 'container-1').map((one) => one.id)).toEqual(['it-1']); + expect(component.stepError('root-1', 'container-1')).toBeNull(); + }); + it('filters out GUARD subflows from a LoopContainer iteration list', () => { retrieveStepIterations.mockReturnValue(of([ iteration('main-1', 1, 'MAIN'), diff --git a/src/app/shared/execution-tree/execution-tree.ts b/src/app/shared/execution-tree/execution-tree.ts index 70c1abd..3f99317 100644 --- a/src/app/shared/execution-tree/execution-tree.ts +++ b/src/app/shared/execution-tree/execution-tree.ts @@ -10,11 +10,14 @@ import { signal, untracked } from '@angular/core'; -import { TaskExecution, TaskExecutionStep } from '@models/task-execution'; +import { getExecutionStatusGroup, TaskExecution, TaskExecutionStep } from '@models/task-execution'; import { ContainersService } from '@services/containers/containers'; import { TaskExecutionsService } from '@services/task-executions/task-executions'; import { take } from 'rxjs'; +/** Joins the two ids a loaded list is keyed by, and splits them again when refreshing it. */ +const STEP_KEY_SEPARATOR = '::'; + export type ExecutionTreeSelection = { executionId: string; }; @@ -80,6 +83,7 @@ export class ExecutionTreeComponent { private readonly stepErrors = signal>({}); private readonly iterationsByStep = signal>({}); private lastRootId: string | null = null; + private lastRootStatus: string | null = null; readonly hasContainerContent = computed(() => { const root = this.rootExecution(); @@ -101,6 +105,51 @@ export class ExecutionTreeComponent { this.stepErrors.set({}); }); }); + + /* + * Keeps the loaded iteration lists fresh while the run is live. + * + * A container spawns one child per iteration, and the endpoint that lists them is the only + * source both for *which* children exist and for what state each is in. Fetching it once, when + * the step was expanded, meant the tree kept showing the run as it was at that instant: a + * three-iteration run displayed "Iteration 1, RUNNING" from start to finish, so a run that was + * working looked stuck and then looked like it had done one iteration and produced nothing. + * + * Polling hands us a new root object every tick, which is the signal to refresh. The last tick + * matters too - it is the one carrying the final status - so a status change is refreshed even + * when it is the transition into a final state. + */ + effect(() => { + const root = this.rootExecution(); + if (!root) return; + const status = String(root.context?.status ?? ''); + const live = getExecutionStatusGroup(status) !== 'FINAL'; + const statusChanged = status !== this.lastRootStatus; + this.lastRootStatus = status; + if (!live && !statusChanged) return; + untracked(() => this.refreshLoadedIterations()); + }); + } + + /** + * Refetches every list already loaded, leaving the previous values in place until the new ones + * arrive so the tree does not blink, and skipping anything already in flight. + */ + private refreshLoadedIterations() { + for (const key of Object.keys(this.iterationsByStep())) { + if (this.loadingStepKeys().has(key)) continue; + const separator = key.indexOf(STEP_KEY_SEPARATOR); + if (separator < 0) continue; + const executionId = key.slice(0, separator); + const stepId = key.slice(separator + STEP_KEY_SEPARATOR.length); + this.taskExecutionsService.retrieveStepIterations(executionId, stepId).pipe(take(1)).subscribe({ + next: (iterations) => this.iterationsByStep + .update((current) => ({ ...current, [key]: iterations })), + // A failed refresh keeps what was on screen: the run itself is unaffected, and replacing a + // list with an error would hide iterations that are still perfectly valid. + error: () => undefined + }); + } } containerSteps(execution: TaskExecution): ContainerStepView[] { @@ -245,6 +294,6 @@ export class ExecutionTreeComponent { } private stepKey(executionId: string, stepId: string): string { - return `${executionId}::${stepId}`; + return `${executionId}${STEP_KEY_SEPARATOR}${stepId}`; } }