From bba4e2e8eeb40b5dd4d115810179708dae5791aa Mon Sep 17 00:00:00 2001 From: kary zheng Date: Tue, 25 Aug 2026 20:01:46 -0700 Subject: [PATCH] feat(gui): gate workflow execution on the compilation state The run button's enabled state came from ValidationWorkflowService alone, which checks an operator's properties against its JSON schema and whether every input port is connected. Neither reads the compiling service, so a workflow whose operators fail to compile was still offered for execution, and pressing run started an execution that reported the same compilation error in the console. Subscribe the menu to the compiling service's state stream and treat a failed compilation the same way as a validation error: the button reads "Invalid Workflow" and is disabled, and runWorkflow returns early. An HTTP failure of the compile endpoint yields no state change rather than a failed one, so a compiling service that is down cannot disable the button. The context menu's "execute to this operator" runs the target together with everything upstream of it, so it is disabled when any operator in the target's sub-DAG failed to compile, not only the target itself. Closes #7981 Co-Authored-By: Claude Opus 5 (1M context) --- .../component/menu/menu.component.spec.ts | 55 +++++++++++++++++++ .../component/menu/menu.component.ts | 22 +++++++- .../context-menu.component.spec.ts | 40 +++++++++++++- .../context-menu/context-menu.component.ts | 19 ++++++- 4 files changed, 130 insertions(+), 6 deletions(-) diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index f9f2994fc06..f7ba00a182b 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -55,6 +55,8 @@ import { GuiConfigService } from "../../../common/service/gui-config.service"; import { MockGuiConfigService } from "../../../common/service/gui-config.service.mock"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; import { UserProjectService } from "../../../dashboard/service/user/project/user-project.service"; +import { WorkflowCompilingService } from "../../service/compile-workflow/workflow-compiling.service"; +import { CompilationState } from "../../types/workflow-compiling.interface"; import type { Mocked } from "vitest"; vi.mock("file-saver", () => ({ saveAs: vi.fn() })); @@ -72,8 +74,10 @@ describe("MenuComponent", () => { let notificationService: NotificationService; let location: Location; let validationStream$: BehaviorSubject; + let compilationStream$: Subject; beforeEach(async () => { + compilationStream$ = new Subject(); await TestBed.configureTestingModule({ imports: [MenuComponent, HttpClientTestingModule, RouterTestingModule.withRoutes([]), NzModalModule], providers: [ @@ -90,6 +94,11 @@ describe("MenuComponent", () => { }, }, { provide: UserService, useClass: StubUserService }, + { + // stubbed so the debounced compile request of the real service does not outlive the test injector + provide: WorkflowCompilingService, + useValue: { getCompilationStateInfoChangedStream: () => compilationStream$.asObservable() }, + }, ...commonTestProviders, ], }).compileComponents(); @@ -131,6 +140,18 @@ describe("MenuComponent", () => { expect(behavior.disable).toBe(true); }); + it("returns 'Invalid Workflow' when the workflow does not compile", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.isWorkflowCompilable = false; + + const behavior = component.getRunButtonBehavior(); + + expect(behavior.text).toBe("Invalid Workflow"); + expect(behavior.icon).toBe("warning"); + expect(behavior.disable).toBe(true); + }); + it("returns 'Empty Workflow' when the workflow has no operators", () => { component.isWorkflowValid = true; component.isWorkflowEmpty = true; @@ -416,6 +437,19 @@ describe("MenuComponent", () => { expect(component.computingUnitSelectionComponent.showAddComputeUnitModalVisible).not.toHaveBeenCalled(); }); + it("does nothing when the workflow does not compile", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.isWorkflowCompilable = false; + component.computingUnitStatus = ComputingUnitState.Running; + const executeSpy = vi.spyOn(executeWorkflowService, "executeWorkflowWithEmailNotification"); + + component.runWorkflow(); + + expect(executeSpy).not.toHaveBeenCalled(); + expect(component.computingUnitSelectionComponent.showAddComputeUnitModalVisible).not.toHaveBeenCalled(); + }); + it("does nothing when the workflow is empty", () => { component.isWorkflowValid = true; component.isWorkflowEmpty = true; @@ -1393,6 +1427,27 @@ describe("MenuComponent", () => { } }); + it("re-applies the run button behavior on every compilation state event", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.computingUnitStatus = ComputingUnitState.Running; + component.executionState = ExecutionState.Uninitialized; + Object.defineProperty(component.workflowWebsocketService, "isConnected", { + get: () => true, + configurable: true, + }); + + compilationStream$.next(CompilationState.Failed); + expect(component.isWorkflowCompilable).toBe(false); + expect(component.runButtonText).toBe("Invalid Workflow"); + expect(component.runDisable).toBe(true); + + compilationStream$.next(CompilationState.Succeeded); + expect(component.isWorkflowCompilable).toBe(true); + expect(component.runButtonText).toBe("Run"); + expect(component.runDisable).toBe(false); + }); + it("deactivates the export button unless the feature is on and results exist", () => { const guiConfig = TestBed.inject(GuiConfigService); const results$ = component.workflowResultExportService.hasResultToExportOnAllOperators; diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index b3e44057da9..662440b788f 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -71,6 +71,8 @@ import { NzSwitchComponent } from "ng-zorro-antd/switch"; import { NzBadgeComponent } from "ng-zorro-antd/badge"; import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; +import { WorkflowCompilingService } from "../../service/compile-workflow/workflow-compiling.service"; +import { CompilationState } from "../../types/workflow-compiling.interface"; /** * MenuComponent is the top level menu bar that shows @@ -127,6 +129,9 @@ export class MenuComponent implements OnInit, OnDestroy { public ComputingUnitState = ComputingUnitState; // make Angular HTML access enum definition public isWorkflowValid: boolean = true; // this will check whether the workflow error or not public isWorkflowEmpty: boolean = false; + // whether the last compilation of the workflow failed. A workflow that cannot compile cannot be executed, + // so it is treated the same way as one that fails the schema / port validation. + public isWorkflowCompilable: boolean = true; public isSaving: boolean = false; public isWorkflowModifiable: boolean = false; public workflowId?: number; @@ -169,6 +174,7 @@ export class MenuComponent implements OnInit, OnDestroy { private location: Location, public undoRedoService: UndoRedoService, public validationWorkflowService: ValidationWorkflowService, + private workflowCompilingService: WorkflowCompilingService, public workflowPersistService: WorkflowPersistService, public workflowVersionService: WorkflowVersionService, public userService: UserService, @@ -233,6 +239,16 @@ export class MenuComponent implements OnInit, OnDestroy { this.applyRunButtonBehavior(this.getRunButtonBehavior()); }); + // the compilation errors are reported per operator on the canvas and in the result panel, but a workflow + // that cannot compile cannot be executed either, so the run button has to reflect the compilation state too + this.workflowCompilingService + .getCompilationStateInfoChangedStream() + .pipe(untilDestroyed(this)) + .subscribe(state => { + this.isWorkflowCompilable = state !== CompilationState.Failed; + this.applyRunButtonBehavior(this.getRunButtonBehavior()); + }); + // Subscribe to WorkflowResultExportService observable this.workflowResultExportService .getExportOnAllOperatorsStatusStream() @@ -353,8 +369,8 @@ export class MenuComponent implements OnInit, OnDestroy { disable: boolean; onClick: () => void; } { - // If workflow is invalid, always disable and show "Invalid Workflow" - if (!this.isWorkflowValid) { + // If workflow is invalid or does not compile, always disable and show "Invalid Workflow" + if (!this.isWorkflowValid || !this.isWorkflowCompilable) { return { text: "Invalid Workflow", icon: "warning", @@ -784,7 +800,7 @@ export class MenuComponent implements OnInit, OnDestroy { */ runWorkflow(): void { // Use the existing flags that were already updated via subscriptions - if (!this.isWorkflowValid || this.isWorkflowEmpty) { + if (!this.isWorkflowValid || !this.isWorkflowCompilable || this.isWorkflowEmpty) { return; } diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts index 2903abc1fb7..e180ae358c1 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts @@ -33,6 +33,7 @@ import { ReactiveFormsModule } from "@angular/forms"; import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { NzDropDownModule } from "ng-zorro-antd/dropdown"; import { ValidationWorkflowService } from "src/app/workspace/service/validation/validation-workflow.service"; +import { WorkflowCompilingService } from "src/app/workspace/service/compile-workflow/workflow-compiling.service"; import { NzModalModule, NzModalService } from "ng-zorro-antd/modal"; import { commonTestProviders } from "../../../../../common/testing/test-utils"; // Import NzModalModule and NzModalService import type { Mocked } from "vitest"; @@ -56,6 +57,7 @@ describe("ContextMenuComponent", () => { let operatorMenuService: Mocked; let jointGraphWrapperSpy: Mocked; let validationWorkflowService: Mocked; + let workflowCompilingService: Mocked; let highlightedOperatorsSubject: BehaviorSubject; let highlightedCommentBoxesSubject: BehaviorSubject; @@ -71,7 +73,12 @@ describe("ContextMenuComponent", () => { jointGraphWrapperSpy.getCurrentHighlightedCommentBoxIDs.mockReturnValue([]); jointGraphWrapperSpy.getCurrentHighlightedLinkIDs.mockReturnValue([]); - const texeraGraphSpy = { isOperatorDisabled: vi.fn(), hasLinkWithID: vi.fn(), bundleActions: vi.fn() }; + const texeraGraphSpy = { + isOperatorDisabled: vi.fn(), + hasLinkWithID: vi.fn(), + bundleActions: vi.fn(), + getSubDAG: vi.fn(), + }; const workflowActionServiceSpy = { getJointGraphWrapper: vi.fn(), @@ -92,6 +99,7 @@ describe("ContextMenuComponent", () => { // Set up TexeraGraph spy return values texeraGraphSpy.hasLinkWithID.mockReturnValue(false); + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [], links: [] }); texeraGraphSpy.bundleActions.mockImplementation((callback: Function) => callback()); const workflowResultServiceSpy = { getResultService: vi.fn(), hasAnyResult: vi.fn() }; @@ -121,6 +129,9 @@ describe("ContextMenuComponent", () => { const validationWorkflowServiceSpy = { validateOperator: vi.fn() }; + const workflowCompilingServiceSpy = { getWorkflowCompilationErrors: vi.fn() }; + workflowCompilingServiceSpy.getWorkflowCompilationErrors.mockReturnValue({}); + await TestBed.configureTestingModule({ providers: [ { provide: OperatorMetadataService, useClass: StubOperatorMetadataService }, @@ -129,6 +140,7 @@ describe("ContextMenuComponent", () => { { provide: WorkflowResultExportService, useValue: workflowResultExportServiceSpy }, { provide: OperatorMenuService, useValue: operatorMenuService }, { provide: ValidationWorkflowService, useValue: validationWorkflowServiceSpy }, + { provide: WorkflowCompilingService, useValue: workflowCompilingServiceSpy }, NzModalService, // Provide NzModalService ...commonTestProviders, ], @@ -151,6 +163,7 @@ describe("ContextMenuComponent", () => { validationWorkflowService = TestBed.inject( ValidationWorkflowService ) as unknown as Mocked; + workflowCompilingService = TestBed.inject(WorkflowCompilingService) as unknown as Mocked; fixture = TestBed.createComponent(ContextMenuComponent); component = fixture.componentInstance; @@ -263,6 +276,31 @@ describe("ContextMenuComponent", () => { expect(texeraGraphSpy.isOperatorDisabled).toHaveBeenCalledWith("op1"); }); + it("should return false when the target operator failed to compile", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [{ operatorID: "op1" }], links: [] } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ op1: {} as any }); + + expect(component.canExecuteOperator()).toBe(false); + expect(texeraGraphSpy.getSubDAG).toHaveBeenCalledWith("op1"); + }); + + it("should return false when an upstream operator failed to compile", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ + operators: [{ operatorID: "op1" }, { operatorID: "upstream" }], + links: [], + } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ upstream: {} as any }); + + expect(component.canExecuteOperator()).toBe(false); + }); + + it("should return true when the compilation error is outside the target's sub-DAG", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [{ operatorID: "op1" }], links: [] } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ unrelated: {} as any }); + + expect(component.canExecuteOperator()).toBe(true); + }); + it("should check disabled status only for valid operators", () => { // First test with invalid operator validationWorkflowService.validateOperator.mockReturnValue({ isValid: false, messages: {} }); diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts index 019f77f7afa..8f0f6321bca 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts @@ -26,6 +26,7 @@ import { WorkflowResultExportService } from "src/app/workspace/service/workflow- import { NzModalService } from "ng-zorro-antd/modal"; import { ResultExportationComponent } from "../../../result-exportation/result-exportation.component"; import { ValidationWorkflowService } from "src/app/workspace/service/validation/validation-workflow.service"; +import { WorkflowCompilingService } from "src/app/workspace/service/compile-workflow/workflow-compiling.service"; import { GuiConfigService } from "../../../../../common/service/gui-config.service"; import { NzMenuDirective, NzMenuItemComponent } from "ng-zorro-antd/menu"; import { NgIf } from "@angular/common"; @@ -51,7 +52,8 @@ export class ContextMenuComponent { protected config: GuiConfigService, private workflowResultService: WorkflowResultService, private modalService: NzModalService, - private validationWorkflowService: ValidationWorkflowService + private validationWorkflowService: ValidationWorkflowService, + private workflowCompilingService: WorkflowCompilingService ) { this.registerWorkflowModifiableChangedHandler(); this.operatorMenuService.highlightedOperators$ @@ -82,10 +84,23 @@ export class ContextMenuComponent { private isOperatorExecutable(operatorID: string): boolean { return ( this.validationWorkflowService.validateOperator(operatorID).isValid && - !this.workflowActionService.getTexeraGraph().isOperatorDisabled(operatorID) + !this.workflowActionService.getTexeraGraph().isOperatorDisabled(operatorID) && + this.isSubDAGCompilable(operatorID) ); } + /** + * Executing to an operator runs the operator together with everything upstream of it, so the entry has to be + * disabled when any operator in that sub-DAG failed to compile, not only when the target operator itself did. + */ + private isSubDAGCompilable(operatorID: string): boolean { + const compilationErrors = this.workflowCompilingService.getWorkflowCompilationErrors(); + return this.workflowActionService + .getTexeraGraph() + .getSubDAG(operatorID) + .operators.every(operator => !(operator.operatorID in compilationErrors)); + } + public hasHighlightedLinks(): boolean { return this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedLinkIDs().length > 0; }