From dcf97dd05ecadc484f000a157399b4485729efb1 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sat, 5 Sep 2026 12:32:45 -0700 Subject: [PATCH 1/3] feat(gui): run the workflow from the Form View and show the author's instruction Add the Run button whose label, icon and disabled state mirror the operator canvas (Invalid / Empty / Connecting / Connect / Run / Stop, in the same precedence), the computing-unit selector, a run clock counted off the engine's own duration event, and a plain-language failure message that collapses opaque SQL/Java traces to one sentence and blames empty required inputs when that is the cause. Running is the same execution call the canvas makes. Show the author's instruction above the inputs as a collapsible, read-only markdown card, only when there is instruction text. Editing it is part of the authoring PR; showing the results follows in the next PR. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --- .../workflow-form.component.html | 86 +++++- .../workflow-form.component.scss | 173 +++++++++++ .../workflow-form.component.spec.ts | 292 +++++++++++++++++- .../workflow-form/workflow-form.component.ts | 290 ++++++++++++++++- .../workflow-form.rendered.spec.ts | 144 ++++++++- .../workflow-form.spec-harness.ts | 47 ++- 6 files changed, 1018 insertions(+), 14 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html index c924532d055..d0c0ce0f0ee 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html @@ -70,6 +70,39 @@
+ +
+ + +
+
+
+
+
Inputs @@ -108,8 +141,57 @@
- + +
+
+ + + +
+ +
+ + + {{ executionDuration | date: "H:mm:ss" : "UTC" }} + +
+
+ +

+ {{ runError || "Running -- this keeps going if you look away." }} +

+ +
diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss index 54e7ffd7bc5..93ef44ad831 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss @@ -190,6 +190,82 @@ $shell: #fafafa; background: #fff; } +/* ---------- instruction ---------- */ + +.instr { + margin-bottom: 26px; + + .instr-bar { + display: flex; + align-items: center; + gap: 10px; + width: 100%; + padding: 13px 16px; + cursor: pointer; + user-select: none; + appearance: none; + border: 0; + background: none; + color: inherit; + font: inherit; + text-align: left; + + h2 { + margin: 0; + font-size: 15px; + font-weight: 600; + flex: 1; + } + + .lead { + color: $blue; + } + + .chev { + color: $text-2; + transform: rotate(-90deg); + transition: transform 0.2s; + } + } + + &.open .instr-bar .chev { + transform: rotate(0deg); + } + + .instr-body { + border-top: 1px solid $divider; + padding: 16px; + /* A long explanation scrolls rather than pushing the form off screen, and can be dragged + taller by anyone who wants to read it all at once. */ + max-height: 340px; + overflow-y: auto; + resize: vertical; + } +} + +.md { + :first-child { + margin-top: 0; + } + + :last-child { + margin-bottom: 0; + } + + img { + max-width: 100%; + border: 1px solid $divider; + border-radius: 6px; + } + + code { + background: $divider; + padding: 1px 6px; + border-radius: 4px; + font-size: 12.5px; + } +} + /* ---------- inputs ---------- */ .pc-section-head { @@ -311,6 +387,103 @@ $shell: #fafafa; } } +/* ---------- run ---------- */ + +.runbar { + display: flex; + align-items: center; + margin-top: 26px; +} + +/* Run and its computing unit as one control. The seam between them is what says the two belong + together. */ +.run-group { + display: inline-flex; + align-items: center; + gap: 12px; + + /* Quiet beside Run: it is something you glance at, not a control. Tabular figures stop the row + twitching as the seconds tick over. */ + .run-clock { + align-self: center; + margin-left: 4px; + color: #5c6672; + font-size: 13px; + font-variant-numeric: tabular-nums; + letter-spacing: 0.02em; + } + + .run-unit { + display: flex; + align-items: center; + + /* The selector reserves 220px and right-aligns inside it, which on the toolbar keeps a row of + controls steady but here left a wide gap between Run and a box that belongs next to it. Let + it be as wide as its content. */ + ::ng-deep .computing-units-selection { + min-width: 0; + } + + /* Matched to Run's height so the pair reads as one row of controls. */ + ::ng-deep .computing-units-dropdown-button { + height: 40px; + } + } +} + +.run { + height: 40px; + // Fixed width, content centred, so the button is the same size in every state (Run / Stop / + // Connect / Connecting / Invalid / Empty) instead of shrinking to "Run" and jumping wide for the + // others. Sized to the longest label ("Connecting"); the disabled states are kept to one word so + // this stays compact. + min-width: 160px; + padding: 0 26px; + font-size: 15px; + font-weight: 600; + border-radius: 8px; + border: 1px solid $blue; + background: $blue; + color: #fff; + display: inline-flex; + align-items: center; + justify-content: center; + gap: 9px; + cursor: pointer; + + &:hover { + background: #40a9ff; + border-color: #40a9ff; + } + + &.stop { + background: #fff; + color: #ff4d4f; + border-color: #ff4d4f; + + &:hover { + background: #fff1f0; + } + } + + &:disabled { + background: $shell; + border-color: $border; + color: rgba(0, 0, 0, 0.25); + cursor: not-allowed; + } +} + +.run-note { + font-size: 13px; + color: $text-2; + margin: 9px 0 0; + + &.err { + color: #ff4d4f; + } +} + /* ---------- workflow preview ---------- */ .wf { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index 543eb940057..d3ba936e211 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -17,7 +17,7 @@ * under the License. */ -import { FormControl } from "@angular/forms"; +import { FormArray, FormControl, FormGroup, Validators } from "@angular/forms"; import { Router } from "@angular/router"; import { of, throwError } from "rxjs"; @@ -26,6 +26,8 @@ import { setupHarness, formViewWorkflow, resolved } from "./workflow-form.spec-h import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; import { DefaultView } from "../../../dashboard/type/workflow-metadata.interface"; import { FORM_DEBOUNCE_TIME_MS } from "../../service/execute-workflow/execute-workflow.service"; +import { ExecutionState } from "../../types/execute-workflow.interface"; +import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; /** * These exercise the page's own decisions -- what a reader is shown, where an ordinary @@ -56,14 +58,17 @@ describe("WorkflowFormComponent", () => { h.workflowResultService as any, h.notificationService as any, h.userService as any, + h.markdownService as any, h.formlyJsonschema as any, h.cdr as any, h.dynamicSchemaService as any, h.workflowCompilingService as any, h.computingUnitStatusService as any, h.workflowConsoleService as any, + h.workflowWebsocketService as any, h.host as any, h.datePipe as any, + h.validationWorkflowService as any, h.config as any ); return component; @@ -882,4 +887,289 @@ describe("WorkflowFormComponent", () => { document.body.removeChild(editable); }); }); + + describe("the author's instruction", () => { + it("shows the instruction as rendered markdown when there is one", async () => { + formBindingService.getConfig.mockReturnValue({ + instruction: { title: "Read me", body: "**bold**" }, + fields: [], + resultOperatorIds: [], + }); + build(formViewWorkflow).ngOnInit(); + // renderInstruction resolves the parsed markdown on a microtask; let it settle. + await Promise.resolve(); + + expect(component.hasInstruction).toBe(true); + expect(component.instructionTitle).toBe("Read me"); + // The markdown mock returns its input; the point is renderInstruction populated the html. + expect(component.instructionPreviewHtml).toBe("**bold**"); + }); + + it("has no instruction when the body is blank", async () => { + formBindingService.getConfig.mockReturnValue({ + instruction: { title: "T", body: " " }, + fields: [], + resultOperatorIds: [], + }); + build(formViewWorkflow).ngOnInit(); + await Promise.resolve(); + + expect(component.hasInstruction).toBe(false); + expect(component.instructionPreviewHtml).toBe(""); + }); + + it("discards a stale instruction render when the body changed while parsing", async () => { + build(formViewWorkflow).ngOnInit(); + (component as any).instructionBody = "first"; + const pending = (component as any).renderInstruction(); + // A newer readConfig sets a different body before the parse microtask resolves. + (component as any).instructionBody = "second"; + await pending; + + // The stale "first" result is dropped rather than overwriting the newer body's render. + expect(component.instructionPreviewHtml).not.toBe("first"); + }); + + it("toggles the instruction open and closed", () => { + build(formViewWorkflow).ngOnInit(); + expect(component.instructionOpen).toBe(true); + + component.toggleInstruction(); + + expect(component.instructionOpen).toBe(false); + }); + }); + + describe("the run button, mirroring the operator canvas", () => { + // Put the page in a ready-to-run state: a WRITE-access unit is up, the socket is connected, the + // graph valid. + const makeReady = () => { + h.workflowWebsocketService.isConnected = true; + h.statusStream.next(ComputingUnitState.Running); + (component as any).selectedUnit = { accessPrivilege: "WRITE" }; + h.validationStream.next({ errors: {}, workflowEmpty: false }); + }; + + it("stores the selected unit from the status service so Run can gate on write access", () => { + build(formViewWorkflow).ngOnInit(); + + h.selectedUnitStream.next({ accessPrivilege: "WRITE" }); + + expect((component as any).selectedUnit).toEqual({ accessPrivilege: "WRITE" }); + }); + + it("offers Connect before a unit is chosen", () => { + build(formViewWorkflow).ngOnInit(); + + expect(component.runButtonState).toEqual({ label: "Connect", icon: "plus-circle", disabled: true }); + }); + + it("offers Run once a unit is up and the graph is valid", () => { + build(formViewWorkflow).ngOnInit(); + makeReady(); + + expect(component.runButtonState.label).toBe("Run"); + expect(component.runButtonState.disabled).toBe(false); + }); + + it("shows Stop while running", () => { + build(formViewWorkflow).ngOnInit(); + h.executionStateStream.next({ current: { state: ExecutionState.Running } }); + + expect(component.isRunning).toBe(true); + expect(component.runButtonState).toEqual({ label: "Stop", icon: "stop", disabled: false }); + }); + + it("disables and says Invalid for a broken graph", () => { + build(formViewWorkflow).ngOnInit(); + makeReady(); + h.validationStream.next({ errors: { op: {} }, workflowEmpty: false }); + + expect(component.runButtonState).toEqual({ label: "Invalid", icon: "warning", disabled: true }); + }); + + it("disables and says Empty for an empty graph", () => { + build(formViewWorkflow).ngOnInit(); + makeReady(); + h.validationStream.next({ errors: {}, workflowEmpty: true }); + + expect(component.runButtonState).toEqual({ label: "Empty", icon: "info-circle", disabled: true }); + }); + + it("disables and says Connecting while the unit's socket comes up", () => { + build(formViewWorkflow).ngOnInit(); + h.statusStream.next(ComputingUnitState.Running); + h.validationStream.next({ errors: {}, workflowEmpty: false }); + h.workflowWebsocketService.isConnected = false; + + expect(component.runButtonState).toEqual({ label: "Connecting", icon: "loading", disabled: true }); + }); + + it("disables with No access when the chosen unit is shared read-only", () => { + build(formViewWorkflow).ngOnInit(); + h.workflowWebsocketService.isConnected = true; + h.statusStream.next(ComputingUnitState.Running); + h.validationStream.next({ errors: {}, workflowEmpty: false }); + (component as any).selectedUnit = { accessPrivilege: "READ" }; + + expect(component.runButtonState).toEqual({ label: "No access", icon: "lock", disabled: true }); + }); + + it("does not offer a dead Stop when the socket drops mid-run", () => { + build(formViewWorkflow).ngOnInit(); + h.statusStream.next(ComputingUnitState.Running); // a unit is selected + h.executionStateStream.next({ current: { state: ExecutionState.Running } }); // a run is in flight + h.workflowWebsocketService.isConnected = false; // its socket drops + + // Still "running", but the button must not offer a Stop that would kill through a dead socket. + expect(component.isRunning).toBe(true); + expect(component.runButtonState).toEqual({ label: "Connecting", icon: "loading", disabled: true }); + }); + + it("repaints when the websocket connection status changes", () => { + build(formViewWorkflow).ngOnInit(); + h.cdr.markForCheck.mockClear(); + + h.connectionStream.next(true); + + expect(h.cdr.markForCheck).toHaveBeenCalled(); + }); + }); + + describe("running", () => { + const makeReady = () => { + h.workflowWebsocketService.isConnected = true; + h.statusStream.next(ComputingUnitState.Running); + (component as any).selectedUnit = { accessPrivilege: "WRITE" }; + h.validationStream.next({ errors: {}, workflowEmpty: false }); + }; + + it("runs the workflow with its name and clears any prior error", () => { + build(formViewWorkflow).ngOnInit(); + makeReady(); + component.runError = "old error"; + + component.onRun(); + + expect(h.executeWorkflowService.executeWorkflow).toHaveBeenCalledWith("scGPT"); + expect(component.runError).toBe(""); + }); + + it("stops a running workflow instead of starting another", () => { + build(formViewWorkflow).ngOnInit(); + h.executionStateStream.next({ current: { state: ExecutionState.Running } }); + + component.onRun(); + + expect(h.executeWorkflowService.killWorkflow).toHaveBeenCalled(); + expect(h.executeWorkflowService.executeWorkflow).not.toHaveBeenCalled(); + }); + + it("does nothing when the button is disabled", () => { + build(formViewWorkflow).ngOnInit(); + // Default state is "Connect" (disabled): no unit chosen. + + component.onRun(); + + expect(h.executeWorkflowService.executeWorkflow).not.toHaveBeenCalled(); + expect(h.executeWorkflowService.killWorkflow).not.toHaveBeenCalled(); + }); + + it("counts the run clock off the engine's duration event", () => { + build(formViewWorkflow).ngOnInit(); + + h.durationEvents.next({ duration: 5000, isRunning: false }); + + expect(component.executionDuration).toBe(5000); + }); + + it("ticks the clock a second at a time while a run is going", () => { + vi.useFakeTimers(); + build(formViewWorkflow).ngOnInit(); + + h.durationEvents.next({ duration: 1000, isRunning: true }); + vi.advanceTimersByTime(1000); + vi.useRealTimers(); + + expect(component.executionDuration).toBe(2000); + }); + }); + + describe("reporting a failed run", () => { + it("blames empty required inputs when a required field is left empty", () => { + build(formViewWorkflow).ngOnInit(); + const form = new FormGroup({ v: new FormControl("", Validators.required) }); + component.rendered = [{ form } as any]; + + h.executionStateStream.next({ current: { state: ExecutionState.Failed, errorMessages: [{ message: "x" }] } }); + + expect(component.runError).toBe("Run failed: please fill in the required fields."); + }); + + it("finds a required error nested inside an array input", () => { + build(formViewWorkflow).ngOnInit(); + const form = new FormGroup({ arr: new FormArray([new FormControl("", Validators.required)]) }); + component.rendered = [{ form } as any]; + + h.executionStateStream.next({ current: { state: ExecutionState.Failed, errorMessages: [{ message: "x" }] } }); + + expect(component.runError).toBe("Run failed: please fill in the required fields."); + }); + + it("does not blame required fields for a non-required validation error", () => { + build(formViewWorkflow).ngOnInit(); + // A pattern failure, not an empty required field: the reader gets the engine message, not + // "fill in the required fields". + const form = new FormGroup({ v: new FormControl("abc", Validators.pattern(/^\d+$/)) }); + component.rendered = [{ form } as any]; + + h.executionStateStream.next({ current: { state: ExecutionState.Failed, errorMessages: [{ message: "boom" }] } }); + + expect(component.runError).toBe("Run failed: boom"); + }); + + it("keeps a short human message, dropping the exception prefix", () => { + build(formViewWorkflow).ngOnInit(); + + h.executionStateStream.next({ + current: { + state: ExecutionState.Failed, + errorMessages: [{ message: "java.lang.RuntimeException: too many rows" }], + }, + }); + + expect(component.runError).toBe("Run failed: too many rows"); + }); + + it("collapses an opaque engine trace to a reload sentence", () => { + build(formViewWorkflow).ngOnInit(); + + h.executionStateStream.next({ + current: { + state: ExecutionState.Failed, + errorMessages: [{ message: "org.jooq.DataAccessException: SQL [..]" }], + }, + }); + + expect(component.runError).toBe("Run failed -- please reload and try again."); + }); + + it("collapses an empty error message to the reload sentence too", () => { + build(formViewWorkflow).ngOnInit(); + + h.executionStateStream.next({ current: { state: ExecutionState.Failed, errorMessages: [] } }); + + expect(component.runError).toBe("Run failed -- please reload and try again."); + }); + + it("gives a generic tail when the message cleans down to nothing", () => { + build(formViewWorkflow).ngOnInit(); + + h.executionStateStream.next({ + current: { state: ExecutionState.Failed, errorMessages: [{ message: "requirement failed: " }] }, + }); + + expect(component.runError).toBe("Run failed: please check your inputs and try again."); + }); + }); }); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index 260f9c23031..f1070a7ccf2 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -19,7 +19,7 @@ import { ChangeDetectorRef, Component, ElementRef, HostListener, OnDestroy, OnInit } from "@angular/core"; import { CommonModule, DatePipe } from "@angular/common"; -import { FormGroup, FormsModule, ReactiveFormsModule } from "@angular/forms"; +import { AbstractControl, FormArray, FormGroup, FormsModule, ReactiveFormsModule } from "@angular/forms"; import { FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; import { FormlyJsonschema } from "@ngx-formly/core/json-schema"; import { ActivatedRoute, Router } from "@angular/router"; @@ -28,12 +28,15 @@ import { NzAvatarModule } from "ng-zorro-antd/avatar"; import { NzIconModule } from "ng-zorro-antd/icon"; import { UserIconComponent } from "../../../dashboard/component/user/user-icon/user-icon.component"; import { cloneDeep } from "lodash-es"; -import { forkJoin, Subject } from "rxjs"; -import { debounceTime, takeUntil } from "rxjs/operators"; +import { MarkdownService } from "ngx-markdown"; +import { EMPTY, forkJoin, Subject, timer } from "rxjs"; +import { debounceTime, switchMap, takeUntil, tap } from "rxjs/operators"; import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; import { FormFieldBinding, Workflow, WorkflowContent } from "../../../common/type/workflow"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; +import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; +import { DashboardWorkflowComputingUnit } from "../../../common/type/workflow-computing-unit"; import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service"; import { NotificationService } from "../../../common/service/notification/notification.service"; import { UserService } from "../../../common/service/user/user.service"; @@ -44,10 +47,14 @@ import { ExecuteWorkflowService, FORM_DEBOUNCE_TIME_MS } from "../../service/exe import { OperatorMetadataService } from "../../service/operator-metadata/operator-metadata.service"; import { FormBindingService, ResolvedField } from "../../service/form-binding/form-binding.service"; import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service"; +import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; import { GuiConfigService } from "../../../common/service/gui-config.service"; import { WorkflowConsoleService } from "../../service/workflow-console/workflow-console.service"; import { WorkflowResultService } from "../../service/workflow-result/workflow-result.service"; +import { WorkflowWebsocketService } from "../../service/workflow-websocket/workflow-websocket.service"; +import { ExecutionState } from "../../types/execute-workflow.interface"; import { Point } from "../../types/workflow-common.interface"; +import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; import { WorkflowEditorComponent } from "../workflow-editor/workflow-editor.component"; import { MiniMapComponent } from "../workflow-editor/mini-map/mini-map.component"; import { CoeditorUserIconComponent } from "../menu/coeditor-user-icon/coeditor-user-icon.component"; @@ -72,8 +79,10 @@ interface RenderedField { * operator's own formly field, so a file property gets the real picker and an attribute a column * dropdown -- and writes a filled-in value straight back to its operator, the same edit the canvas * makes, with each sub-field of a nested or repeated property renamed and hidden as the author set - * it up. Running the workflow and showing results are added by later PRs. A view, not a new object: - * it opens the same workflow the canvas does. + * it up. It also shows the author's instruction above the inputs, and runs the workflow: a Run + * button (a reader's simplified Run/Stop, sharing the canvas's disable conditions), the + * computing-unit selector, a run clock and plain-language failure messages. Showing the results is + * added by a later PR. A view, not a new object: it opens the same workflow the canvas does. */ @UntilDestroy() @Component({ @@ -88,6 +97,7 @@ interface RenderedField { NzAvatarModule, NzIconModule, UserIconComponent, + ComputingUnitSelectionComponent, WorkflowEditorComponent, MiniMapComponent, CoeditorUserIconComponent, @@ -108,6 +118,31 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { /** Torn down and replaced whenever the form is rebuilt, so an old field's write-back stops. */ private formsRebuilt = new Subject(); + /** The author's instruction, rendered above the inputs; the reader always sees it as markdown. */ + public instructionOpen = true; + public instructionTitle = ""; + public instructionBody = ""; + public instructionPreviewHtml = ""; + + /** + * Milliseconds the current run has been going, counted from the same engine event the operator + * canvas counts: the engine reports the real elapsed time, and a local 1s timer fills in between + * reports so the display ticks instead of jumping. + */ + public executionDuration = 0; + public executionState: ExecutionState = ExecutionState.Uninitialized; + public runError = ""; + /** The picked unit's connection state, mirrored from the same stream the operator canvas reads, + * so "Connecting" here means exactly what it means there. */ + public computingUnitStatus: ComputingUnitState = ComputingUnitState.NoComputingUnit; + /** The picked unit itself, kept so Run can be gated on write access to it -- the same gate the + * operator canvas applies (a READ/NONE-shared unit can be viewed but not executed on). */ + private selectedUnit: DashboardWorkflowComputingUnit | null = null; + /** Workflow validity, read from the same validation stream the operator canvas uses, so Run is + * disabled ("Invalid" / "Empty") in the same cases. */ + public isWorkflowValid = true; + public isWorkflowEmpty = false; + /** The collapsible workflow preview: closed until the reader opens it. */ public workflowOpen = false; /** The embedded canvas is built the first time the strip opens, never while collapsed. */ @@ -137,6 +172,7 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { private workflowResultService: WorkflowResultService, private notificationService: NotificationService, private userService: UserService, + private markdownService: MarkdownService, private formlyJsonschema: FormlyJsonschema, private cdr: ChangeDetectorRef, // Injected for its side effect: it fills its map from the operator-add stream, so it has to @@ -150,8 +186,12 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { private workflowCompilingService: WorkflowCompilingService, private computingUnitStatusService: ComputingUnitStatusService, private workflowConsoleService: WorkflowConsoleService, + private workflowWebsocketService: WorkflowWebsocketService, private host: ElementRef, private datePipe: DatePipe, + // Same source the operator canvas reads its "Invalid" / "Empty" states from, so Run is + // disabled here exactly when it is disabled there. + private validationWorkflowService: ValidationWorkflowService, private config: GuiConfigService ) {} @@ -164,6 +204,77 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { this.wid = wid; this.load(wid); + // The run clock, reusing the operator canvas's source outright rather than timing anything + // here: the engine is the only thing that knows when the run really began, so a stopwatch + // started at the click would drift and would be wrong after a reload. + this.workflowWebsocketService + .subscribeToEvent("ExecutionDurationUpdateEvent") + .pipe( + tap(event => (this.executionDuration = event.duration)), + switchMap(event => (event.isRunning ? timer(1000, 1000) : EMPTY)), + untilDestroyed(this) + ) + .subscribe(() => { + this.executionDuration += 1000; + this.cdr.markForCheck(); + }); + + // The run button's state is read from getters, so a change in unit/connection/validity has to + // repaint the view. markForCheck, not detectChanges: a synchronous pass can be thrown out of by + // an unrelated component's NG0100, killing the subscription. + this.computingUnitStatusService + .getSelectedComputingUnit() + .pipe(untilDestroyed(this)) + .subscribe(unit => { + this.selectedUnit = unit; + this.cdr.markForCheck(); + }); + this.computingUnitStatusService + .getStatus() + .pipe(untilDestroyed(this)) + .subscribe(status => { + this.computingUnitStatus = status; + this.cdr.markForCheck(); + }); + this.workflowWebsocketService + .getConnectionStatusStream() + .pipe(untilDestroyed(this)) + .subscribe(() => this.cdr.markForCheck()); + // Validity from the canvas's own stream, so a broken graph disables Run ("Invalid") here + // exactly as it does there. + this.validationWorkflowService + .getWorkflowValidationErrorStream() + .pipe(untilDestroyed(this)) + .subscribe(value => { + this.isWorkflowEmpty = value.workflowEmpty; + this.isWorkflowValid = Object.keys(value.errors).length === 0; + this.cdr.markForCheck(); + }); + + this.executeWorkflowService + .getExecutionStateStream() + .pipe(untilDestroyed(this)) + .subscribe(({ current }) => { + this.executionState = current.state; + // Surface a failed run. Without this the spinner just stops and the form gives zero + // feedback -- the opposite of what a reader needs. onRun() clears runError before the next + // run, so a stale error never lingers. + if (current.state === ExecutionState.Failed) { + // A required input left empty is by far the commonest reason a run fails here, and the + // engine reports it as an opaque "... is not contained in the schema". Answer with the + // same word the field itself already shows ("required"), so the two messages are + // consistent -- and it covers every operator, not just this one. + this.runError = this.hasEmptyRequiredInputs() + ? "Run failed: please fill in the required fields." + : this.friendlyRunError(current.errorMessages?.[0]?.message?.trim() ?? ""); + } + // markForCheck, not detectChanges: this is the one subscription the page cannot afford to + // lose. A synchronous detectChanges can be thrown out of by an unrelated component's NG0100, + // which would kill this stream and freeze the Run button on a stale state with no error + // shown; marking dirty and letting the next pass render avoids that. + this.cdr.markForCheck(); + }); + // Attribute boxes become dropdowns only after compilation writes the column enums into each // operator's dynamic schema -- which lands after these cards were built. Rebuild on the // compilation-state stream, a ReplaySubject(1) so a late subscriber (this page reloads fresh @@ -261,7 +372,12 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { } private readConfig(): void { + const config = this.formBindingService.getConfig(); this.parameters = this.formBindingService.resolveFields(); + this.instructionTitle = config.instruction?.title ?? ""; + this.instructionBody = config.instruction?.body ?? ""; + // A reader always sees the instruction as rendered markdown. + void this.renderInstruction(); this.buildForm(); } @@ -511,6 +627,170 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { return rendered.resolved.binding.id; } + // --------------------------------------------------------------------------- + // Instruction: the author's one piece of guidance, shown as rendered markdown + // --------------------------------------------------------------------------- + + public get hasInstruction(): boolean { + return this.instructionBody.trim().length > 0; + } + + private async renderInstruction(): Promise { + // Capture the body this render is for: parsing can resolve on a later microtask, and a fresh + // readConfig() may start another render meanwhile. If the configured body changed while we were + // parsing, this result is stale -- drop it so the newer render's output stands. + const body = this.instructionBody; + const html = body.trim() ? await Promise.resolve(this.markdownService.parse(body)) : ""; + if (body !== this.instructionBody) { + return; + } + this.instructionPreviewHtml = html; + this.cdr.detectChanges(); + } + + public toggleInstruction(): void { + this.instructionOpen = !this.instructionOpen; + } + + // --------------------------------------------------------------------------- + // Running the same workflow the canvas runs, through the same execute/kill service. The canvas + // wraps its run with completion-email options (executeWorkflowWithEmailNotification); this page + // runs plainly (executeWorkflow), so a form-started run does not send that email. + // --------------------------------------------------------------------------- + + public get isRunning(): boolean { + return ( + this.executionState !== ExecutionState.Uninitialized && + this.executionState !== ExecutionState.Completed && + this.executionState !== ExecutionState.Failed && + this.executionState !== ExecutionState.Killed && + this.executionState !== ExecutionState.Terminated + ); + } + + /** + * A unit is picked but its socket is still coming up -- the same window the operator canvas shows + * "Connecting" and disables its run button. Read from the exact condition the canvas uses + * (menu.component's getRunButtonBehavior), so the two stay in step. + */ + public get isConnecting(): boolean { + return ( + this.computingUnitStatus !== ComputingUnitState.NoComputingUnit && !this.workflowWebsocketService.isConnected + ); + } + + /** No unit chosen yet: the button shows a disabled "Connect" hint and the unit is picked in the + * embedded selector -- unlike the canvas, where the Connect button is itself the click target. */ + public get hasNoComputingUnit(): boolean { + return this.computingUnitStatus === ComputingUnitState.NoComputingUnit; + } + + /** Write access to the chosen unit: the canvas gates execution on this (a READ/NONE-shared unit + * can be selected and viewed but not run on), so the form must too, or a reader could execute on + * a unit they only have read access to. */ + public get hasUnitWriteAccess(): boolean { + return this.selectedUnit?.accessPrivilege === "WRITE"; + } + + /** + * The Run button's label, icon and disabled state. It shares the operator canvas's disable + * conditions -- an invalid or empty workflow, a unit still connecting, or no unit chosen each + * disable it and say why -- but deliberately simplifies the execution states a reader needs down + * to Run and Stop, with no pause/resume: while a run is in flight the button stops (kills) it, + * otherwise it runs. (The canvas offers Pause/Resume/Submitting and a clickable Connect; a form + * reader does not, and picks the unit in the embedded selector instead.) + */ + public get runButtonState(): { label: string; icon: string; disabled: boolean } { + // Connecting is checked before Stop on purpose: if the socket drops mid-run, a "Stop" would + // send killWorkflow() through a dead socket and do nothing, so a disconnected unit disables the + // button (as the canvas does) rather than offering a kill that cannot be delivered. + if (this.isConnecting) { + return { label: "Connecting", icon: "loading", disabled: true }; + } + if (this.isRunning) { + return { label: "Stop", icon: "stop", disabled: false }; + } + if (!this.isWorkflowValid) { + return { label: "Invalid", icon: "warning", disabled: true }; + } + if (this.isWorkflowEmpty) { + return { label: "Empty", icon: "info-circle", disabled: true }; + } + if (this.hasNoComputingUnit) { + return { label: "Connect", icon: "plus-circle", disabled: true }; + } + // A unit is chosen and connected, but shared to this reader read-only: the canvas gates + // execution on write access to the unit, so the form disables Run rather than sending a request + // that the unit would reject. + if (!this.hasUnitWriteAccess) { + return { label: "No access", icon: "lock", disabled: true }; + } + return { label: "Run", icon: "caret-right", disabled: false }; + } + + public onRun(): void { + if (this.isRunning) { + this.executeWorkflowService.killWorkflow(); + return; + } + // The button is disabled in exactly the states a run cannot start from (invalid/empty workflow, + // connecting, or no unit), so a stray call here would be a silent no-op. + if (this.runButtonState.disabled) { + return; + } + this.runError = ""; + // Run as-is, like the canvas -- no client-side "fill everything first" gate (it diverged from + // the canvas and could not guarantee success anyway). Empty/invalid inputs surface as a real + // engine error via the execution-state stream (see the Failed handler). + this.executeWorkflowService.executeWorkflow(this.workflowName); + } + + /** + * Turn an engine error into something a reader can act on: raw SQL/jOOQ/Java traces collapse to + * one plain sentence, a short human message is kept (minus any Java prefix). The full text is + * always logged for developers. + */ + private friendlyRunError(raw: string): string { + if (raw) { + // eslint-disable-next-line no-console + console.error("[workflow-form] run failed:", raw); + } + const opaque = + !raw || /\bSQL \[|org\.jooq|org\.apache|org\.postgresql|foreign key|constraint|jdbc|\bat [\w.$]+\(/i.test(raw); + if (opaque) { + return "Run failed -- please reload and try again."; + } + const cleaned = raw + .replace(/^[\w.$]+(?:Exception|Error):\s*/, "") + .replace(/^requirement failed:\s*/i, "") + .trim(); + return `Run failed: ${cleaned || "please check your inputs and try again."}`; + } + + /** + * Whether any exposed input that is required is still empty. Reuses formly's own per-field + * required validation -- the very thing that renders "This field is required" under the box -- so + * the run-failure message stays consistent with the field hint. + */ + private hasEmptyRequiredInputs(): boolean { + // Specifically a `required` error (a required field left empty), not just any invalid control: + // a pattern or range failure is a different problem and should not be answered with "fill in the + // required fields". Walk the control tree for a real required error. + const hasRequiredError = (control: AbstractControl): boolean => { + if (control.hasError("required")) { + return true; + } + if (control instanceof FormGroup) { + return Object.values(control.controls).some(hasRequiredError); + } + if (control instanceof FormArray) { + return control.controls.some(hasRequiredError); + } + return false; + }; + return this.rendered.some(r => hasRequiredError(r.form)); + } + /** Open or close the workflow preview; opening it builds the canvas the first time. */ public toggleWorkflow(): void { this.workflowOpen = !this.workflowOpen; diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts index d6e6a4cb93a..bca2ee4e046 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts @@ -23,6 +23,17 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { ActivatedRoute, Router } from "@angular/router"; import { FormlyForm, FormlyModule } from "@ngx-formly/core"; import { FormlyJsonschema } from "@ngx-formly/core/json-schema"; +import { NZ_ICONS } from "ng-zorro-antd/icon"; +import { + InfoCircleOutline, + DownOutline, + PlusCircleOutline, + CaretRightOutline, + StopOutline, + WarningOutline, + LoadingOutline, + LockOutline, +} from "@ant-design/icons-angular/icons"; import { EMPTY, of, Subject } from "rxjs"; import { WorkflowFormComponent } from "./workflow-form.component"; @@ -39,8 +50,18 @@ import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-w import { WorkflowResultService } from "../../service/workflow-result/workflow-result.service"; import { NotificationService } from "../../../common/service/notification/notification.service"; import { UserService } from "../../../common/service/user/user.service"; +import { MarkdownService } from "ngx-markdown"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { WorkflowConsoleService } from "../../service/workflow-console/workflow-console.service"; +import { WorkflowWebsocketService } from "../../service/workflow-websocket/workflow-websocket.service"; +import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; +import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; +import { WorkflowComputingUnitManagingService } from "../../../common/service/computing-unit/workflow-computing-unit/workflow-computing-unit-managing.service"; +import { WorkflowExecutionsService } from "../../../dashboard/service/user/workflow-executions/workflow-executions.service"; +import { ComputingUnitActionsService } from "../../../common/service/computing-unit/computing-unit-actions/computing-unit-actions.service"; +import { WorkflowPveService } from "../../service/virtual-environment/virtual-environment.service"; +import { NzModalService } from "ng-zorro-antd/modal"; +import { ExecutionState } from "../../types/execute-workflow.interface"; import { GuiConfigService } from "../../../common/service/gui-config.service"; /** @@ -71,6 +92,11 @@ describe("WorkflowFormComponent (rendered template)", () => { // the page's own inputs markup -- the section head, the empty state, the card and the form // wrapper -- rendered and covered. TestBed.overrideComponent(FormlyForm, { set: { template: "" } }); + // Blank the computing-unit selector's own template (a child, not the page): its real markup + // needs a modal/executions/PVE service chain out of scope here. Blanking the child -- rather + // than overriding the page's imports, which would JIT-recompile the page and drop its + // host-binding coverage -- keeps the run bar around it rendered and the page fully covered. + TestBed.overrideComponent(ComputingUnitSelectionComponent, { set: { template: "" } }); /* eslint-enable no-restricted-syntax */ await TestBed.configureTestingModule({ @@ -118,7 +144,17 @@ describe("WorkflowFormComponent (rendered template)", () => { { provide: OperatorMetadataService, useValue: { getOperatorMetadata: () => of({}) } }, { provide: FormBindingService, - useValue: { resolveFields: () => [], readValue: () => undefined, writeValue: vi.fn() }, + useValue: { + // An instruction so the instruction card renders and is covered. + getConfig: () => ({ + instruction: { title: "How to use this", body: "Fill in the inputs." }, + fields: [], + resultOperatorIds: [], + }), + resolveFields: () => [], + readValue: () => undefined, + writeValue: vi.fn(), + }, }, { provide: FormlyJsonschema, useValue: { toFieldConfig: () => ({ fieldGroup: [] }) } }, { provide: DynamicSchemaService, useValue: { getDynamicSchema: () => ({ jsonSchema: {} }) } }, @@ -126,13 +162,58 @@ describe("WorkflowFormComponent (rendered template)", () => { provide: WorkflowCompilingService, useValue: { getCompilationStateInfoChangedStream: () => EMPTY }, }, - { provide: ExecuteWorkflowService, useValue: { resetExecutionAndWorkers: vi.fn() } }, + { + provide: ExecuteWorkflowService, + useValue: { + getExecutionStateStream: () => EMPTY, + executeWorkflow: vi.fn(), + killWorkflow: vi.fn(), + resetExecutionAndWorkers: vi.fn(), + }, + }, { provide: WorkflowResultService, useValue: { clearResults: vi.fn() } }, { provide: NotificationService, useValue: { error: vi.fn() } }, { provide: UserService, useValue: { getCurrentUser: () => undefined, isLogin: () => false } }, - { provide: ComputingUnitStatusService, useValue: { disconnect: vi.fn() } }, + { provide: MarkdownService, useValue: { parse: (s: string) => s } }, + { + provide: ComputingUnitStatusService, + useValue: { + disconnect: vi.fn(), + getSelectedComputingUnit: () => EMPTY, + getStatus: () => EMPTY, + // Read by the (blanked) computing-unit selector's own ngOnInit. + getAllComputingUnits: () => EMPTY, + }, + }, + // The blanked computing-unit selector still constructs and runs ngOnInit; give it the few + // services it reads so it does not throw. It renders nothing (its template is blanked). + { provide: WorkflowComputingUnitManagingService, useValue: { getComputingUnitLimitOptions: () => EMPTY } }, + { provide: WorkflowExecutionsService, useValue: {} }, + { provide: ComputingUnitActionsService, useValue: {} }, + { provide: WorkflowPveService, useValue: {} }, + { provide: NzModalService, useValue: {} }, { provide: WorkflowConsoleService, useValue: { clearConsoleMessages: vi.fn() } }, + { + provide: WorkflowWebsocketService, + useValue: { subscribeToEvent: () => EMPTY, isConnected: true, getConnectionStatusStream: () => EMPTY }, + }, + { provide: ValidationWorkflowService, useValue: { getWorkflowValidationErrorStream: () => EMPTY } }, { provide: GuiConfigService, useValue: { env: { formViewEnabled: true } } }, + // Register the icons the run bar and instruction use, so nz-icon renders them inline instead + // of fetching each SVG over HTTP (an unresolved fetch that would hang fixture.whenStable). + { + provide: NZ_ICONS, + useValue: [ + InfoCircleOutline, + DownOutline, + PlusCircleOutline, + CaretRightOutline, + StopOutline, + WarningOutline, + LoadingOutline, + LockOutline, + ], + }, DatePipe, ], }).compileComponents(); @@ -253,6 +334,63 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(el(".param.read-only")).not.toBeNull(); }); + it("renders the author's instruction card and toggles it", async () => { + fixture.detectChanges(); + finishLoad(); + // renderInstruction resolves the markdown on a microtask. + await fixture.whenStable(); + fixture.detectChanges(); + + expect(el(".card.instr")).not.toBeNull(); + expect(el(".instr .instr-bar h2")?.textContent?.trim()).toBe("How to use this"); + expect(el(".instr .md")?.innerHTML).toContain("Fill in the inputs."); + + (el(".instr-bar") as HTMLButtonElement).click(); + expect(fixture.componentInstance.instructionOpen).toBe(false); + }); + + it("renders the run bar with the run button and the computing-unit selector", () => { + fixture.detectChanges(); + finishLoad(); + + expect(el(".runbar .run")).not.toBeNull(); + // Default state: no unit chosen, so the button reads Connect and is disabled. + expect(el(".runbar .run")?.textContent?.trim()).toContain("Connect"); + expect((el(".runbar .run") as HTMLButtonElement).disabled).toBe(true); + expect(el(".runbar texera-computing-unit-selection")).not.toBeNull(); + // At rest there is nothing to count and no run note. + expect(el(".run-clock")).toBeNull(); + expect(el(".run-note")).toBeNull(); + }); + + it("fires onRun when the enabled run button is clicked", () => { + fixture.detectChanges(); + finishLoad(); + // A running state makes the button "Stop" (enabled); a disabled button would swallow the click. + fixture.componentInstance.executionState = ExecutionState.Running; + fixture.detectChanges(); + const run = vi.spyOn(fixture.componentInstance, "onRun").mockImplementation(() => {}); + + el(".runbar .run")!.click(); + + expect(run).toHaveBeenCalled(); + }); + + it("announces a run failure as an alert and a running note as a status", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + + c.runError = "Run failed: boom"; + fixture.detectChanges(); + expect(el(".run-note")?.getAttribute("role")).toBe("alert"); + + c.runError = ""; + c.executionState = ExecutionState.Running; + fixture.detectChanges(); + expect(el(".run-note")?.getAttribute("role")).toBe("status"); + }); + it("tears the workflow down when the browser unloads (the beforeunload host binding)", () => { fixture.detectChanges(); finishLoad(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index 842cbea78d3..4a9b7fa6055 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts @@ -56,6 +56,15 @@ export function setupHarness() { const workflowMetaDataChangedStream = new Subject(); // Compilation reports column names late; the form rebuilds its inputs off this stream. const compilationChanged = new Subject(); + // Run-related streams the run tests drive: execution state, the engine's duration event, the + // computing-unit connection status, the workflow validity, and the websocket connection. + const executionStateStream = new Subject(); + const durationEvents = new Subject<{ duration: number; isRunning: boolean }>(); + const statusStream = new Subject(); + // The picked computing unit (with its accessPrivilege), separate from the connection status. + const selectedUnitStream = new Subject(); + const validationStream = new Subject<{ errors: Record; workflowEmpty: boolean }>(); + const connectionStream = new Subject(); // The operators the graph holds: `hasOperatorIds` gates operatorSchemaFor, `graphOperators` // supplies each operator's type (which picks the custom widget). Tests add to them as needed. const hasOperatorIds = new Set(); @@ -87,6 +96,9 @@ export function setupHarness() { // Resolves the exposed inputs and reads/writes their values. Tests point `resolveFields` at the // inputs they want rendered; `readValue` seeds the write-back guard. const formBindingService = { + // The presentation config: the instruction plus the fields. Tests override getConfig to give an + // instruction; resolveFields drives which inputs render. + getConfig: vi.fn().mockReturnValue({ instruction: undefined, fields: [], resultOperatorIds: [] }), resolveFields: vi.fn().mockReturnValue([]), readValue: vi.fn().mockReturnValue(undefined), writeValue: vi.fn(), @@ -160,14 +172,34 @@ export function setupHarness() { const coeditorPresenceService = { coeditors: [] }; const route = { snapshot: { params: { id: "7" } } }; const operatorMetadataService = { getOperatorMetadata: () => of({}) }; - const executeWorkflowService = { resetExecutionAndWorkers: vi.fn() }; + const executeWorkflowService = { + getExecutionStateStream: () => executionStateStream.asObservable(), + executeWorkflow: vi.fn(), + killWorkflow: vi.fn(), + resetExecutionAndWorkers: vi.fn(), + }; const workflowResultService = { clearResults: vi.fn() }; const notificationService = { error: vi.fn() }; // Not logged in by default so opening a workflow does not save; the save tests log in. const userService = { getCurrentUser: () => undefined, isLogin: vi.fn().mockReturnValue(false) }; - const cdr = { detectChanges: vi.fn() }; - const computingUnitStatusService = { disconnect: vi.fn() }; + const markdownService = { parse: (s: string) => s }; + const cdr = { detectChanges: vi.fn(), markForCheck: vi.fn() }; + const computingUnitStatusService = { + disconnect: vi.fn(), + getSelectedComputingUnit: () => selectedUnitStream.asObservable(), + getStatus: () => statusStream.asObservable(), + }; const workflowConsoleService = { clearConsoleMessages: vi.fn() }; + // The websocket the run clock and the "Connecting" state read. `isConnected` is a plain settable + // flag so a test can put the page in the connecting window. + const workflowWebsocketService = { + subscribeToEvent: (_: string) => durationEvents.asObservable(), + isConnected: true, + getConnectionStatusStream: () => connectionStream.asObservable(), + }; + const validationWorkflowService = { + getWorkflowValidationErrorStream: () => validationStream.asObservable(), + }; // The name field is measured off the host; querySelector returns null so the measuring // (DOM-layout, jsdom has none) short-circuits. `contains` drives isTypingInTheForm; false by // default so a rebuild is never suppressed, and overridden by the tests that probe typing. @@ -195,18 +227,27 @@ export function setupHarness() { workflowResultService, notificationService, userService, + markdownService, formlyJsonschema, cdr, dynamicSchemaService, workflowCompilingService, computingUnitStatusService, workflowConsoleService, + workflowWebsocketService, + validationWorkflowService, host, datePipe, config, workflowChangedStream, workflowMetaDataChangedStream, compilationChanged, + executionStateStream, + durationEvents, + statusStream, + selectedUnitStream, + validationStream, + connectionStream, hasOperatorIds, graphOperators, triggerCenterEvent, From 8c2b011a710c7304fd9ff1171e2e847dffaafcc8 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sun, 6 Sep 2026 12:36:50 -0700 Subject: [PATCH 2/3] feat(gui): show the chosen results on the Form View Render each chosen, currently-viewed step's output under the workflow that produced it -- a table, a visualization, or a compact "no result yet" -- with a per-result zoom and visualizations fitted to their cards after a run. The chosen set is a pure display filter that follows the canvas's view-result set (getOperatorsToViewResult) and never writes it, per the settled result-panel design. Add WorkflowResultService.hasNonEmptyResult so a view-result step that produced zero tuples (e.g. a download/publish UDF) reads as "no result" rather than an empty card. Opening a step to inspect it, and the authoring mode that picks what to show, follow in later PRs. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --- .../workflow-form.component.html | 70 ++++++ .../workflow-form.component.scss | 127 ++++++++++ .../workflow-form.component.spec.ts | 159 ++++++++++++ .../workflow-form/workflow-form.component.ts | 233 +++++++++++++++++- .../workflow-form.rendered.spec.ts | 52 +++- .../workflow-form.spec-harness.ts | 33 ++- .../workflow-result.service.spec.ts | 22 ++ .../workflow-result.service.ts | 14 ++ 8 files changed, 704 insertions(+), 6 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html index d0c0ce0f0ee..0b268628392 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html @@ -223,5 +223,75 @@

{{ instructionTitle || "How to use this" }}

class="box">
+ + +
+
Results
+

+ {{ isRunning ? "Working…" : hasRunFinished ? "This run produced no results to show." : "Press Run and the + results appear here." }} +

+ + +
+
+ {{ resultLabel(id) }} + + + + + +
+ +
+ + + + + +

+ {{ isRunning ? "Computing…" : "No result yet." }} +

+
+
+ +
diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss index 93ef44ad831..77185232a69 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss @@ -568,3 +568,130 @@ $shell: #fafafa; } } } + +/* ---------- results ---------- */ + +.results { + margin-top: 28px; +} + +/* Quiet on purpose: it marks where the answer will land, and should not compete with the inputs + the reader is still filling in. */ +.results-empty { + margin: 0; + padding: 22px 0 6px; + color: $text-2; + font-size: 13.5px; +} + +.result { + margin-bottom: 14px; + overflow: hidden; + + .result-head { + display: flex; + align-items: center; + justify-content: space-between; + gap: 10px; + padding: 9px 13px; + border-bottom: 1px solid $divider; + font-size: 13px; + font-weight: 600; + background: $shell; + } + + .result-zoom { + display: inline-flex; + gap: 2px; + + button { + border: 1px solid $border; + background: #fff; + border-radius: 5px; + width: 24px; + height: 24px; + display: grid; + place-items: center; + color: $text-2; + cursor: pointer; + font-size: 11px; + + &:hover:not(:disabled) { + color: $blue; + border-color: $blue; + } + + &:disabled { + opacity: 0.35; + cursor: default; + } + } + } + + /* Centred in the card so an empty / computing step reads at the same height as a full one -- + every card is one fixed height, whatever its state. */ + .result-pending { + margin: 0; + height: 100%; + display: grid; + place-items: center; + text-align: center; + color: $text-2; + font-size: 13.5px; + } + + .result-body { + padding: 4px; + /* Every result card is the SAME height -- table, chart, empty, or "computing" -- so the list + never jumps between a tall box and a thin strip. Content scrolls inside; an empty state + centres. Tall enough for a screenful of rows, nowhere near a full-window box. */ + height: 380px; + overflow: auto; + + /* The reused result table's empty state ends with "Tip: enable the eye icon on the operator + ...". There is no such icon on this page and the step is already chosen, so that tip only + misleads here -- drop it, keep the neutral "Empty result set". */ + ::ng-deep texera-result-table-frame h4 + p { + display: none; + } + + /* Centre the table's own "Empty result set" in the card, where the other empty states sit. + Only the empty block carries this inline style, so a populated table is untouched. */ + ::ng-deep texera-result-table-frame > div[style*="text-align"] { + height: 340px; + display: grid; + place-items: center; + } + + /* An iframe has no intrinsic height; fill the card so a chart is the same height as a table. + The zoom control enlarges the picture inside without changing the card. */ + texera-visualization-panel-content { + display: block; + width: 100%; + height: 100%; + } + + /* Zoom scales the picture itself (the iframe's rendered content), not the card frame. The card + keeps its height and scrolls to reach the rest of an enlarged image. */ + &[data-zoom="0"] ::ng-deep iframe { + transform: scale(0.7); + transform-origin: top left; + } + + &[data-zoom="2"] ::ng-deep iframe { + transform: scale(1.6); + transform-origin: top left; + } + + /* A plot with dozens of categories needs the width most: squeezed narrow its axis labels + collapse into an unreadable smear. Let the iframe and whatever it renders take the card. */ + ::ng-deep iframe, + ::ng-deep .visualization-frame, + ::ng-deep .plotly-container, + ::ng-deep .js-plotly-plot { + width: 100% !important; + max-width: none !important; + height: 100% !important; + } + } +} diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index d3ba936e211..67bbec74d3c 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -68,6 +68,7 @@ describe("WorkflowFormComponent", () => { h.workflowWebsocketService as any, h.host as any, h.datePipe as any, + h.panelResizeService as any, h.validationWorkflowService as any, h.config as any ); @@ -1055,6 +1056,26 @@ describe("WorkflowFormComponent", () => { expect(component.runError).toBe(""); }); + it("clears a stale failure banner when a new run starts, even a co-editor's", () => { + build(formViewWorkflow).ngOnInit(); + h.executionStateStream.next({ current: { state: ExecutionState.Failed, errorMessages: [{ message: "boom" }] } }); + expect(component.runError).not.toBe(""); + + // A co-editor starts the next run: the shared stream goes in-flight without this page's onRun(). + h.executionStateStream.next({ current: { state: ExecutionState.Running } }); + + expect(component.runError).toBe(""); + }); + + it("tells apart a never-run form from a completed run that produced nothing", () => { + build(formViewWorkflow).ngOnInit(); + expect(component.hasRunFinished).toBe(false); + + h.executionStateStream.next({ current: { state: ExecutionState.Completed } }); + + expect(component.hasRunFinished).toBe(true); + }); + it("stops a running workflow instead of starting another", () => { build(formViewWorkflow).ngOnInit(); h.executionStateStream.next({ current: { state: ExecutionState.Running } }); @@ -1095,6 +1116,144 @@ describe("WorkflowFormComponent", () => { }); }); + describe("showing the chosen results", () => { + // Chosen operators are only shown if they still have view-result on in the canvas; the form + // never writes that set (display filter, per the settled design). + const chosen = (resultOperatorIds: string[]) => + formBindingService.getConfig.mockReturnValue({ instruction: undefined, fields: [], resultOperatorIds }); + + it("shows a chosen result only while its operator still has view-result on the canvas", () => { + build(formViewWorkflow).ngOnInit(); + chosen(["a", "b"]); + h.viewResultIds.add("a"); // b's eye is off on the canvas + + (component as any).readConfig(); + + expect(component.shownResultIds).toEqual(["a"]); + }); + + it("drops a card when the canvas view-result set changes, without a result update", () => { + build(formViewWorkflow).ngOnInit(); + chosen(["a", "b"]); + h.viewResultIds.add("a"); + h.viewResultIds.add("b"); + (component as any).readConfig(); + expect(component.shownResultIds).toEqual(["a", "b"]); + + // A co-editor turns b's eye off on the canvas. This emits no result-update event, so the + // filter must react to the view-result set changing directly, or b's card would go stale. + h.viewResultIds.delete("b"); + h.viewResultChanged.next({}); + + expect(component.shownResultIds).toEqual(["a"]); + }); + + it("cards only the chosen, viewed steps that actually produced a result", () => { + build(formViewWorkflow).ngOnInit(); + chosen(["produces", "produces-nothing"]); + h.viewResultIds.add("produces"); + h.viewResultIds.add("produces-nothing"); + h.anyResultIds.add("produces"); // the other ran but yielded nothing (e.g. a download UDF) + + (component as any).readConfig(); + + expect(component.resultIdsToShow).toEqual(["produces"]); + expect(component.hasResults).toBe(true); + }); + + it("has no results when nothing chosen has produced anything", () => { + build(formViewWorkflow).ngOnInit(); + chosen(["a"]); + h.viewResultIds.add("a"); + (component as any).readConfig(); + + expect(component.hasResults).toBe(false); + }); + + it("calls a paginated result a table, and gates visualisation content on a snapshot", () => { + build(formViewWorkflow).ngOnInit(); + (component as any).workflowResultService.hasPaginatedResult = (id: string) => id === "tab"; + + expect(component.isTabularResult("tab")).toBe(true); + expect(component.vizHasContent("tab")).toBe(false); // tables take the tabular branch + // A non-tabular op with a non-empty snapshot has viz content; an empty one does not. + h.snapshotById.set("viz", [{ a: 1 }]); + expect(component.vizHasContent("viz")).toBe(true); + expect(component.vizHasContent("blank")).toBe(false); + }); + + it("labels a result by the operator's friendly name, falling back to the id", () => { + build(formViewWorkflow).ngOnInit(); + h.graphOperators.push({ operatorID: "op-1", operatorType: "CSVFileScan" }); + + expect(component.resultLabel("op-1")).toBe("CSVFileScan"); + expect(component.resultLabel("gone")).toBe("gone"); + }); + + it("keeps a result's frame identity stable until its version moves", () => { + build(formViewWorkflow).ngOnInit(); + const before = component.resultKey("op-1"); + expect(component.resultKey("op-1")).toBe(before); + + (component as any).resultVersion.set("op-1", 1); + + expect(component.resultKey("op-1")).not.toBe(before); + expect(component.trackByKey(0, "k")).toBe("k"); + }); + + it("resizes a result within bounds, per result, and re-fits after", () => { + vi.useFakeTimers(); + build(formViewWorkflow).ngOnInit(); + const fit = vi.spyOn(component as any, "fitVisualisations").mockImplementation(() => {}); + expect(component.resultZoom("op-1")).toBe(1); + + component.zoomResult("op-1", 1); + component.zoomResult("op-1", 1); + expect(component.resultZoom("op-1")).toBe(2); // clamped at 2 + + component.zoomResult("op-1", -1); + component.zoomResult("op-1", -1); + component.zoomResult("op-1", -1); + expect(component.resultZoom("op-1")).toBe(0); // clamped at 0 + expect(component.resultZoom("op-2")).toBe(1); // untouched + + // The deferred re-fit runs after the card height lands. + vi.advanceTimersByTime(60); + expect(fit).toHaveBeenCalled(); + vi.useRealTimers(); + }); + + it("bumps the result version and re-fits on a result update", () => { + vi.useFakeTimers(); + build(formViewWorkflow).ngOnInit(); + const fit = vi.spyOn(component as any, "fitVisualisations").mockImplementation(() => {}); + + h.resultUpdateStream.next({ "op-1": {} }); + expect(component.resultKey("op-1")).toBe("op-1#1"); + vi.advanceTimersByTime(300); + expect(fit).toHaveBeenCalled(); + vi.useRealTimers(); + }); + + it("re-fits the charts once a finished run has results", () => { + vi.useFakeTimers(); + build(formViewWorkflow).ngOnInit(); + vi.spyOn(component, "hasResults", "get").mockReturnValue(true); + const fit = vi.spyOn(component as any, "fitVisualisations").mockImplementation(() => {}); + + h.executionStateStream.next({ current: { state: ExecutionState.Completed } }); + vi.advanceTimersByTime(400); + + expect(fit).toHaveBeenCalled(); + vi.useRealTimers(); + }); + + it("gives the result tables a realistic page height on init", () => { + build(formViewWorkflow).ngOnInit(); + expect(h.panelResizeService.changePanelSize).toHaveBeenCalled(); + }); + }); + describe("reporting a failed run", () => { it("blames empty required inputs when a required field is left empty", () => { build(formViewWorkflow).ngOnInit(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index f1070a7ccf2..d92384f164b 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -26,6 +26,8 @@ import { ActivatedRoute, Router } from "@angular/router"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { NzAvatarModule } from "ng-zorro-antd/avatar"; import { NzIconModule } from "ng-zorro-antd/icon"; +import { NzButtonModule } from "ng-zorro-antd/button"; +import { NzTooltipModule } from "ng-zorro-antd/tooltip"; import { UserIconComponent } from "../../../dashboard/component/user/user-icon/user-icon.component"; import { cloneDeep } from "lodash-es"; import { MarkdownService } from "ngx-markdown"; @@ -51,10 +53,13 @@ import { ValidationWorkflowService } from "../../service/validation/validation-w import { GuiConfigService } from "../../../common/service/gui-config.service"; import { WorkflowConsoleService } from "../../service/workflow-console/workflow-console.service"; import { WorkflowResultService } from "../../service/workflow-result/workflow-result.service"; +import { PanelResizeService } from "../../service/workflow-result/panel-resize/panel-resize.service"; import { WorkflowWebsocketService } from "../../service/workflow-websocket/workflow-websocket.service"; import { ExecutionState } from "../../types/execute-workflow.interface"; import { Point } from "../../types/workflow-common.interface"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; +import { ResultTableFrameComponent } from "../result-panel/result-table-frame/result-table-frame.component"; +import { VisualizationFrameContentComponent } from "../visualization-panel-content/visualization-frame-content.component"; import { WorkflowEditorComponent } from "../workflow-editor/workflow-editor.component"; import { MiniMapComponent } from "../workflow-editor/mini-map/mini-map.component"; import { CoeditorUserIconComponent } from "../menu/coeditor-user-icon/coeditor-user-icon.component"; @@ -81,8 +86,11 @@ interface RenderedField { * makes, with each sub-field of a nested or repeated property renamed and hidden as the author set * it up. It also shows the author's instruction above the inputs, and runs the workflow: a Run * button (a reader's simplified Run/Stop, sharing the canvas's disable conditions), the - * computing-unit selector, a run clock and plain-language failure messages. Showing the results is - * added by a later PR. A view, not a new object: it opens the same workflow the canvas does. + * computing-unit selector, a run clock and plain-language failure messages. It then shows the + * chosen results underneath -- a table, a visualisation, or a compact "no result yet" -- as a + * display filter that follows the canvas's view-result set and never writes it. Opening a step to + * inspect it read-only, and the authoring mode that picks what to show, are later PRs. A view, not + * a new object: it opens the same workflow the canvas does. */ @UntilDestroy() @Component({ @@ -96,8 +104,12 @@ interface RenderedField { FormlyModule, NzAvatarModule, NzIconModule, + NzButtonModule, + NzTooltipModule, UserIconComponent, ComputingUnitSelectionComponent, + ResultTableFrameComponent, + VisualizationFrameContentComponent, WorkflowEditorComponent, MiniMapComponent, CoeditorUserIconComponent, @@ -143,6 +155,20 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { public isWorkflowValid = true; public isWorkflowEmpty = false; + /** + * Which steps' results to show, as a display filter over the canvas's view-result set: the + * author's chosen `resultOperatorIds`, kept to only those an operator still has view-result + * ("the eye") on. The form NEVER writes the canvas's view-result flags -- it only limits what it + * itself shows, so a canvas user's result-viewing is unaffected. + */ + public shownResultIds: string[] = []; + /** Chart height per result (0 compact / 1 default / 2 tall). Per operator so one does not resize + * the others, and in memory only -- a viewing preference, not part of the workflow. */ + private zoomByResult = new Map(); + /** Bumped when a result changes, used as the chart's *ngFor identity so the frame is rebuilt, not + * reused: the chart reads its content once at creation, so a stale frame showed "undefined". */ + private resultVersion = new Map(); + /** The collapsible workflow preview: closed until the reader opens it. */ public workflowOpen = false; /** The embedded canvas is built the first time the strip opens, never while collapsed. */ @@ -189,6 +215,10 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { private workflowWebsocketService: WorkflowWebsocketService, private host: ElementRef, private datePipe: DatePipe, + // The result table sizes its rows-per-page from this shared panel height. On the operator + // canvas the docked panel drives it; this page has no such panel, so left at the tiny default + // every table showed a single row per page. Given a realistic height in ngOnInit instead. + private panelResizeService: PanelResizeService, // Same source the operator canvas reads its "Invalid" / "Empty" states from, so Run is // disabled here exactly when it is disabled there. private validationWorkflowService: ValidationWorkflowService, @@ -202,8 +232,39 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { return; } this.wid = wid; + // Give the result tables a realistic height to page against, so they show a screenful of rows + // instead of one. (~7 rows; the card scrolls for the rest.) + this.panelResizeService.changePanelSize(900, 560); this.load(wid); + // A result changing bumps that operator's version (so its chart frame is rebuilt, not reused), + // re-limits what the form shows to the currently-viewed set, and re-fits the visualisations. + this.workflowResultService + .getResultUpdateStream() + .pipe(untilDestroyed(this)) + .subscribe(update => { + for (const operatorID of Object.keys(update ?? {})) { + this.resultVersion.set(operatorID, (this.resultVersion.get(operatorID) ?? 0) + 1); + } + this.refreshShownResults(); + // markForCheck, not detectChanges: this fires often during a run, and a synchronous pass + // can be thrown out of by an unrelated component's NG0100, killing the subscription. + this.cdr.markForCheck(); + this.later(() => this.fitVisualisations(), 300); + }); + + // Turning a step's view-result OFF on the canvas emits no result-update event, so the filter + // above would miss it and leave a stale card. React to the view-result set changing directly + // (a co-editor's toggle included), so a de-viewed step drops out here at once. + this.workflowActionService + .getTexeraGraph() + .getViewResultOperatorsChangedStream() + .pipe(untilDestroyed(this)) + .subscribe(() => { + this.refreshShownResults(); + this.cdr.markForCheck(); + }); + // The run clock, reusing the operator canvas's source outright rather than timing anything // here: the engine is the only thing that knows when the run really began, so a stopwatch // started at the click would drift and would be wrong after a reload. @@ -255,10 +316,17 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { .getExecutionStateStream() .pipe(untilDestroyed(this)) .subscribe(({ current }) => { + const wasRunning = this.isRunning; this.executionState = current.state; + // Clear a stale failure banner the moment any new run starts -- this session's or a + // co-editor's. onRun() clears it for a run started here, but a co-editor's run moves the + // shared execution stream to an in-flight state without going through onRun(), so without + // this the previous failure would linger over their running run. + if (!wasRunning && this.isRunning) { + this.runError = ""; + } // Surface a failed run. Without this the spinner just stops and the form gives zero - // feedback -- the opposite of what a reader needs. onRun() clears runError before the next - // run, so a stale error never lingers. + // feedback -- the opposite of what a reader needs. if (current.state === ExecutionState.Failed) { // A required input left empty is by far the commonest reason a run fails here, and the // engine reports it as an opaque "... is not contained in the schema". Answer with the @@ -268,6 +336,12 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { ? "Run failed: please fill in the required fields." : this.friendlyRunError(current.errorMessages?.[0]?.message?.trim() ?? ""); } + // Fit the charts to their cards once a run has results. Deliberately not on run START: the + // run repaints operators and a re-fit then zoomed the whole preview down. Deliberately does + // not open the workflow either -- someone using the form came for the inputs and results. + if (this.hasResults) { + this.later(() => this.fitVisualisations(), 400); + } // markForCheck, not detectChanges: this is the one subscription the page cannot afford to // lose. A synchronous detectChanges can be thrown out of by an unrelated component's NG0100, // which would kill this stream and freeze the Run button on a stale state with no error @@ -376,11 +450,23 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { this.parameters = this.formBindingService.resolveFields(); this.instructionTitle = config.instruction?.title ?? ""; this.instructionBody = config.instruction?.body ?? ""; + this.refreshShownResults(); // A reader always sees the instruction as rendered markdown. void this.renderInstruction(); this.buildForm(); } + /** + * Limit the shown results to the author's chosen operators that STILL have view-result on in the + * canvas. This is a pure display filter: it reads the canvas's view-result set and never writes + * it, so a normal canvas user's result-viewing is unaffected. A chosen operator whose view-result + * was turned off (or that was deleted) simply drops out here rather than rendering a stale card. + */ + private refreshShownResults(): void { + const viewed = this.workflowActionService.getTexeraGraph().getOperatorsToViewResult(); + this.shownResultIds = this.formBindingService.getConfig().resultOperatorIds.filter(id => viewed.has(id)); + } + /** * Build the form from the operators' JSON schemas (FormlyJsonschema), keeping the one field per * exposed property. Each input gets its own form keyed by binding id. @@ -627,6 +713,131 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { return rendered.resolved.binding.id; } + // --------------------------------------------------------------------------- + // Results: the chosen steps' output, shown under the workflow that produced it + // --------------------------------------------------------------------------- + + public get hasResults(): boolean { + return this.shownResultIds.some(id => this.workflowResultService.hasNonEmptyResult(id)); + } + + /** + * The chosen steps that actually produced a result, so only those get a card. Whether a Python + * UDF yields a result cannot be known from the graph -- some (a download/publish step) never do -- + * so a chosen step earns its card at runtime rather than sitting on a permanent "No result yet.". + */ + public get resultIdsToShow(): string[] { + return this.shownResultIds.filter(id => this.workflowResultService.hasNonEmptyResult(id)); + } + + public isTabularResult(operatorID: string): boolean { + return this.workflowResultService.hasPaginatedResult(operatorID); + } + + /** + * Whether this step's visualisation drew something. A visualiser reserves a fixed canvas even + * when empty, so gating on real content lets an empty result collapse to the compact "No result + * yet" line instead of a tall blank box. Tables are excluded (they take the tabular branch). + */ + public vizHasContent(operatorID: string): boolean { + if (this.isTabularResult(operatorID)) { + return false; + } + const snapshot = this.workflowResultService.getResultService(operatorID)?.getCurrentResultSnapshot(); + return !!snapshot && snapshot.length > 0; + } + + /** The operator's friendly label for a result card, falling back to its raw id. */ + public resultLabel(operatorID: string): string { + const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorID); + return operator ? this.formBindingService.operatorLabel(operator) : operatorID; + } + + public trackByKey(_: number, key: string): string { + return key; + } + + /** + * A per-result identity that changes on each result-update for that operator, used as the chart's + * *ngFor key so the frame is rebuilt (not reused) when the result changes: the chart reads its + * content once at creation, so a reused frame kept showing the old (or "undefined") picture. A + * visualisation's result arrives as a single snapshot, so in practice this bumps about once per + * result rather than per tuple. + */ + public resultKey(operatorID: string): string { + return operatorID + "#" + (this.resultVersion.get(operatorID) ?? 0); + } + + public resultZoom(operatorID: string): number { + return this.zoomByResult.get(operatorID) ?? 1; + } + + public zoomResult(operatorID: string, delta: number): void { + const next = Math.min(2, Math.max(0, this.resultZoom(operatorID) + delta)); + this.zoomByResult.set(operatorID, next); + // Let the new card height land, then have the chart redraw into it -- growing the frame alone + // leaves the picture at its old size until something asks it to re-measure. + this.cdr.detectChanges(); + this.later(() => this.fitVisualisations(), 60); + } + + /** + * Scale each visualisation to its card. They render in a same-origin srcdoc iframe at natural + * size, so we inject a stylesheet to fit the content to the card and fire a resize so chart + * libraries re-lay out. The operator's output is untouched. + */ + /* v8 ignore start -- iframe/Plotly DOM fitting; no coverage in jsdom */ + private fitVisualisations(): void { + const frames = this.host.nativeElement.querySelectorAll(".result-body iframe"); + frames.forEach(frame => { + const apply = () => { + try { + const doc = frame.contentDocument; + if (!doc?.body) { + return; + } + if (!doc.getElementById("pc-fit")) { + const style = doc.createElement("style"); + style.id = "pc-fit"; + style.textContent = ` + html, body { margin: 0; padding: 8px; overflow-x: hidden; } + .js-plotly-plot, .plot-container, .plotly, .svg-container { width: 100% !important; height: 100% !important; } + img, svg, canvas, video { max-width: 100% !important; height: auto !important; } + table { max-width: 100%; } + `; + doc.head?.appendChild(style); + } + const win = frame.contentWindow as (Window & { Plotly?: any }) | null; + const plots = doc.querySelectorAll(".js-plotly-plot"); + if (win?.Plotly?.Plots?.resize && plots.length) { + plots.forEach(plot => { + plot.style.width = "100%"; + plot.style.height = "100%"; + try { + win.Plotly.Plots.resize(plot); + } catch { + // A chart mid-render cannot be resized; the next call will catch it. + } + }); + } + win?.dispatchEvent(new Event("resize")); + } catch { + // A cross-origin document cannot be styled from here; leave it as it came. + } + }; + apply(); + // Re-apply after the iframe (re)loads. Bind once per frame element (guarded by a data flag): + // a `{ once: true }` listener added on every fit call never fires for an already-loaded frame, + // so repeated zoom/fit calls would pile up detached listeners. A single persistent listener + // per frame re-fits on each reload and is torn down with the frame. + if (!frame.dataset.pcFitBound) { + frame.dataset.pcFitBound = "1"; + frame.addEventListener("load", apply); + } + }); + } + /* v8 ignore stop */ + // --------------------------------------------------------------------------- // Instruction: the author's one piece of guidance, shown as rendered markdown // --------------------------------------------------------------------------- @@ -668,6 +879,20 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { ); } + /** + * A run has started and ended, as opposed to never having run. Lets the empty results section say + * "this run produced nothing" after a completed-but-empty run, instead of the "press Run" hint + * that wrongly implies nothing has run yet. + */ + public get hasRunFinished(): boolean { + return ( + this.executionState === ExecutionState.Completed || + this.executionState === ExecutionState.Failed || + this.executionState === ExecutionState.Killed || + this.executionState === ExecutionState.Terminated + ); + } + /** * A unit is picked but its socket is still coming up -- the same window the operator canvas shows * "Connecting" and disables its run button. Read from the exact condition the canvas uses diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts index bca2ee4e046..ede84a90fa1 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts @@ -33,6 +33,8 @@ import { WarningOutline, LoadingOutline, LockOutline, + MinusOutline, + PlusOutline, } from "@ant-design/icons-angular/icons"; import { EMPTY, of, Subject } from "rxjs"; @@ -56,6 +58,9 @@ import { WorkflowConsoleService } from "../../service/workflow-console/workflow- import { WorkflowWebsocketService } from "../../service/workflow-websocket/workflow-websocket.service"; import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; +import { ResultTableFrameComponent } from "../result-panel/result-table-frame/result-table-frame.component"; +import { VisualizationFrameContentComponent } from "../visualization-panel-content/visualization-frame-content.component"; +import { PanelResizeService } from "../../service/workflow-result/panel-resize/panel-resize.service"; import { WorkflowComputingUnitManagingService } from "../../../common/service/computing-unit/workflow-computing-unit/workflow-computing-unit-managing.service"; import { WorkflowExecutionsService } from "../../../dashboard/service/user/workflow-executions/workflow-executions.service"; import { ComputingUnitActionsService } from "../../../common/service/computing-unit/computing-unit-actions/computing-unit-actions.service"; @@ -97,6 +102,11 @@ describe("WorkflowFormComponent (rendered template)", () => { // than overriding the page's imports, which would JIT-recompile the page and drop its // host-binding coverage -- keeps the run bar around it rendered and the page fully covered. TestBed.overrideComponent(ComputingUnitSelectionComponent, { set: { template: "" } }); + // Blank the two result frame children: the real table/visualization need a live result service + // and (for the chart) an iframe jsdom cannot run. Blanking the children keeps the page's own + // results markup -- the section, the card, the head, the zoom controls -- rendered and covered. + TestBed.overrideComponent(ResultTableFrameComponent, { set: { template: "" } }); + TestBed.overrideComponent(VisualizationFrameContentComponent, { set: { template: "" } }); /* eslint-enable no-restricted-syntax */ await TestBed.configureTestingModule({ @@ -130,6 +140,9 @@ describe("WorkflowFormComponent (rendered template)", () => { triggerCenterEvent: vi.fn(), hasOperator: () => false, getOperator: () => undefined, + getAllOperators: () => [], + getOperatorsToViewResult: () => new Set(), + getViewResultOperatorsChangedStream: () => EMPTY, }), }, }, @@ -171,7 +184,18 @@ describe("WorkflowFormComponent (rendered template)", () => { resetExecutionAndWorkers: vi.fn(), }, }, - { provide: WorkflowResultService, useValue: { clearResults: vi.fn() } }, + { + provide: WorkflowResultService, + useValue: { + clearResults: vi.fn(), + getResultUpdateStream: () => EMPTY, + hasNonEmptyResult: () => false, + hasAnyResult: () => false, + hasPaginatedResult: () => false, + getResultService: () => undefined, + }, + }, + { provide: PanelResizeService, useValue: { changePanelSize: vi.fn() } }, { provide: NotificationService, useValue: { error: vi.fn() } }, { provide: UserService, useValue: { getCurrentUser: () => undefined, isLogin: () => false } }, { provide: MarkdownService, useValue: { parse: (s: string) => s } }, @@ -212,6 +236,8 @@ describe("WorkflowFormComponent (rendered template)", () => { WarningOutline, LoadingOutline, LockOutline, + MinusOutline, + PlusOutline, ], }, DatePipe, @@ -391,6 +417,30 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(el(".run-note")?.getAttribute("role")).toBe("status"); }); + it("renders the results section: empty state, then a result card for a produced step", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + + // Before any result: the section shows its quiet empty line, no cards. + expect(el(".results .label")?.textContent?.trim()).toBe("Results"); + expect(el(".results-empty")).not.toBeNull(); + expect(el(".result")).toBeNull(); + + // A chosen step reports a non-empty result: a card appears. Kept in the neutral "no result + // yet" switch branch (not tabular, no snapshot) so the heavy table/visualization children -- + // which their own specs cover, and which drag in a websocket/status chain jsdom cannot run -- + // are not instantiated here; this test covers the page's own card + head markup. + const wrs: any = TestBed.inject(WorkflowResultService); + wrs.hasNonEmptyResult = () => true; + c.shownResultIds = ["op-1"]; + fixture.detectChanges(); + + expect(el(".results-empty")).toBeNull(); + expect(el(".result .result-head")).not.toBeNull(); + expect(el(".result .result-body")).not.toBeNull(); + }); + it("tears the workflow down when the browser unloads (the beforeunload host binding)", () => { fixture.detectChanges(); finishLoad(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index 4a9b7fa6055..e74487327b7 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts @@ -65,10 +65,18 @@ export function setupHarness() { const selectedUnitStream = new Subject(); const validationStream = new Subject<{ errors: Record; workflowEmpty: boolean }>(); const connectionStream = new Subject(); + // A result changed; the form bumps versions and re-fits. Tests drive it directly. + const resultUpdateStream = new Subject>(); + // Fires when the canvas's view-result set changes (a co-editor's eye toggle included). + const viewResultChanged = new Subject(); // The operators the graph holds: `hasOperatorIds` gates operatorSchemaFor, `graphOperators` // supplies each operator's type (which picks the custom widget). Tests add to them as needed. const hasOperatorIds = new Set(); const graphOperators: any[] = []; + // Operators with view-result ("the eye") on in the canvas -- the ONLY ones the form may show. + const viewResultIds = new Set(); + // Operators that produced a non-empty result -- drives hasNonEmptyResult in the result mock. + const anyResultIds = new Set(); // The preview centres the embedded graph once it is built; tests assert this fired. const triggerCenterEvent = vi.fn(); @@ -89,6 +97,9 @@ export function setupHarness() { triggerCenterEvent, hasOperator: (id: string) => hasOperatorIds.has(id), getOperator: (id: string) => graphOperators.find(o => o.operatorID === id), + getAllOperators: () => graphOperators, + getOperatorsToViewResult: () => new Set(viewResultIds), + getViewResultOperatorsChangedStream: () => viewResultChanged.asObservable(), }), // Exposing or un-exposing a property announces on this stream; the form re-reads its config. formBindingChanged$: new Subject(), @@ -102,6 +113,8 @@ export function setupHarness() { resolveFields: vi.fn().mockReturnValue([]), readValue: vi.fn().mockReturnValue(undefined), writeValue: vi.fn(), + // A result card's friendly label; the mock returns the operator's display name or its id. + operatorLabel: (op: any) => op?.customDisplayName ?? op?.operatorType ?? op?.operatorID, }; // A field per property the tests expose. Real formly json-schema conversion is exercised by the // property panel's own spec; here a deterministic map keeps these tests about the component's @@ -178,7 +191,19 @@ export function setupHarness() { killWorkflow: vi.fn(), resetExecutionAndWorkers: vi.fn(), }; - const workflowResultService = { clearResults: vi.fn() }; + // Results: `anyResultIds` marks which operators produced a non-empty result; `snapshotById` + // lets a test give an operator a snapshot (drives vizHasContent). `hasPaginatedResult` is off + // unless a test overrides it. `getResultUpdateStream` is the stream the form watches. + const snapshotById = new Map>(); + const workflowResultService = { + clearResults: vi.fn(), + getResultUpdateStream: () => resultUpdateStream.asObservable(), + hasNonEmptyResult: (id: string) => anyResultIds.has(id), + hasAnyResult: (id: string) => anyResultIds.has(id), + hasPaginatedResult: (_id: string) => false, + getResultService: (id: string) => ({ getCurrentResultSnapshot: () => snapshotById.get(id) }), + }; + const panelResizeService = { changePanelSize: vi.fn() }; const notificationService = { error: vi.fn() }; // Not logged in by default so opening a workflow does not save; the save tests log in. const userService = { getCurrentUser: () => undefined, isLogin: vi.fn().mockReturnValue(false) }; @@ -235,6 +260,7 @@ export function setupHarness() { computingUnitStatusService, workflowConsoleService, workflowWebsocketService, + panelResizeService, validationWorkflowService, host, datePipe, @@ -248,8 +274,13 @@ export function setupHarness() { selectedUnitStream, validationStream, connectionStream, + resultUpdateStream, + viewResultChanged, hasOperatorIds, graphOperators, + viewResultIds, + anyResultIds, + snapshotById, triggerCenterEvent, }; } diff --git a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts index 31b83b5b109..e53de2c11fc 100644 --- a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts +++ b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts @@ -115,6 +115,28 @@ describe("WorkflowResultService", () => { expect(updateEvents).toEqual([updates]); }); + it("hasNonEmptyResult tells an empty result apart from one with rows", () => { + const ws = TestBed.inject(WorkflowWebsocketService); + pushWsEvent(ws, { + type: "WebResultUpdateEvent", + updates: { + emptyPag: paginationUpdate(0), + fullPag: paginationUpdate(5), + emptySnap: snapshotUpdate([]), + fullSnap: snapshotUpdate([{ a: 1 }]), + }, + tableStats: {}, + }); + + // A service exists for the empty step (hasAnyResult), but it holds nothing. + expect(service.hasAnyResult("emptyPag")).toBe(true); + expect(service.hasNonEmptyResult("emptyPag")).toBe(false); + expect(service.hasNonEmptyResult("fullPag")).toBe(true); + expect(service.hasNonEmptyResult("emptySnap")).toBe(false); + expect(service.hasNonEmptyResult("fullSnap")).toBe(true); + expect(service.hasNonEmptyResult("never-ran")).toBe(false); + }); + it("announces newly-created operators on the result-initiate stream", () => { const ws = TestBed.inject(WorkflowWebsocketService); const initiated: string[] = []; diff --git a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts index 89d4e453fab..e7879ba15e8 100644 --- a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts +++ b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts @@ -64,6 +64,20 @@ export class WorkflowResultService { return this.hasResult(operatorID) || this.hasPaginatedResult(operatorID); } + /** + * Whether the operator produced an actual, non-empty result. A step marked view-result still + * registers a (paginated) result service with zero tuples -- e.g. a UDF that only writes a file + * or logs -- so hasAnyResult alone is true for those. This checks the tuple count / snapshot + * length so an empty result reads as "no result". + */ + public hasNonEmptyResult(operatorID: string): boolean { + const paginated = this.getPaginatedResultService(operatorID); + if (paginated) { + return paginated.getCurrentTotalNumTuples() > 0; + } + return (this.getResultService(operatorID)?.getCurrentResultSnapshot()?.length ?? 0) > 0; + } + public hasResult(operatorID: string): boolean { return isDefined(this.getResultService(operatorID)); } From 387bf0eebb63ce28802d136ff582b0b18cea260b Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sun, 6 Sep 2026 12:56:25 -0700 Subject: [PATCH 3/3] feat(gui): open a step to inspect its settings read-only on the Form View Clicking a step on the embedded preview opens its property panel read-only: the panel carries the inert attribute, which blocks pointer AND keyboard AND focus (the graph is modification-disabled too), and it stays the scroll container so a long panel is still readable. Clicking empty canvas dismisses it, and the form stays silent on the shared co-editor channel so it never shows as editing a step. Turning the panel live to choose what to expose is the authoring PR. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --- ...ator-property-edit-frame.component.spec.ts | 32 ++++++ .../operator-property-edit-frame.component.ts | 20 +++- .../property-editor.component.spec.ts | 31 ++++++ .../property-editor.component.ts | 44 +++++++-- .../workflow-form.component.html | 34 +++++++ .../workflow-form.component.scss | 84 ++++++++++++++++ .../workflow-form.component.spec.ts | 97 +++++++++++++++++++ .../workflow-form/workflow-form.component.ts | 79 ++++++++++++++- .../workflow-form.rendered.spec.ts | 31 ++++++ .../workflow-form.spec-harness.ts | 19 ++++ 10 files changed, 460 insertions(+), 11 deletions(-) diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts index 5326030d78a..2d2e6bd4a5e 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts @@ -161,6 +161,38 @@ describe("OperatorPropertyEditFrameComponent", () => { expect(component).toBeTruthy(); }); + it("broadcasts currentlyEditing and syncs the operator version by default when an operator opens", () => { + workflowActionService.addOperator(mockScanPredicate, mockPoint); + const spy = vi.spyOn(workflowActionService.getTexeraGraph(), "updateSharedModelAwareness"); + const versionSpy = vi.spyOn(workflowActionService, "setOperatorVersion"); + + component.ngOnChanges({ + currentOperatorId: new SimpleChange(undefined, mockScanPredicate.operatorID, true), + }); + fixture.detectChanges(); + + expect(spy).toHaveBeenCalledWith("currentlyEditing", mockScanPredicate.operatorID); + expect(versionSpy).toHaveBeenCalledWith(mockScanPredicate.operatorID, expect.anything()); + }); + + it("neither broadcasts nor mutates the operator version when broadcastEditing is false (read-only inspect)", () => { + // The Form View mounts this frame with broadcastEditing=false so a reader inspecting a step is + // not announced as editing the graph, and opening the step does not write the operator version + // into the shared model. + workflowActionService.addOperator(mockScanPredicate, mockPoint); + component.broadcastEditing = false; + const spy = vi.spyOn(workflowActionService.getTexeraGraph(), "updateSharedModelAwareness"); + const versionSpy = vi.spyOn(workflowActionService, "setOperatorVersion"); + + component.ngOnChanges({ + currentOperatorId: new SimpleChange(undefined, mockScanPredicate.operatorID, true), + }); + fixture.detectChanges(); + + expect(spy).not.toHaveBeenCalledWith("currentlyEditing", mockScanPredicate.operatorID); + expect(versionSpy).not.toHaveBeenCalled(); + }); + /** * test if the property editor correctly receives the operator highlight stream, * get the operator data (id, property, and metadata), and then display the form. diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts index be354825fa0..d3fa940ee05 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts @@ -178,6 +178,11 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On /** True while an author is choosing which properties appear on the Form View; adds a tick * box beside each. Off, the property editor is unchanged. */ @Input() exposeChoosing = false; + /** Whether opening an operator may write to the shared workflow: the "currently editing" co-editor + * broadcast AND the operator-version sync (both are shared-model writes). True on the operator + * canvas; the Form View sets it false to inspect a step read-only, so opening one neither shows + * the reader as a co-editor nor mutates the operator's version. */ + @Input() broadcastEditing = true; currentOperatorSchema?: OperatorSchema; @@ -631,10 +636,19 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On this.currentOperatorSchema = this.dynamicSchemaService.getDynamicSchema(this.currentOperatorId); this.currentOperatorStatus = this.workflowStatusSerivce.getCurrentStatus()[this.currentOperatorId]; - this.workflowActionService.getTexeraGraph().updateSharedModelAwareness("currentlyEditing", this.currentOperatorId); + if (this.broadcastEditing) { + this.workflowActionService + .getTexeraGraph() + .updateSharedModelAwareness("currentlyEditing", this.currentOperatorId); + } const operator = this.workflowActionService.getTexeraGraph().getOperator(this.currentOperatorId); - // set the operator data needed - this.workflowActionService.setOperatorVersion(operator.operatorID, this.currentOperatorSchema.operatorVersion); + // Syncing the operator to the current schema version writes the new version into the Yjs shared + // model (changeOperatorVersion), which broadcasts and persists. That is right on the canvas, but + // a read-only inspect (broadcastEditing=false) must not mutate the workflow just by opening a + // step, so skip the sync there and show the version as stored. + if (this.broadcastEditing) { + this.workflowActionService.setOperatorVersion(operator.operatorID, this.currentOperatorSchema.operatorVersion); + } this.operatorVersion = operator.operatorVersion.slice(0, 9); this.setFormlyFormBinding(this.currentOperatorSchema.jsonSchema); this.formTitle = operator.customDisplayName ?? this.currentOperatorSchema.additionalMetadata.userFriendlyName; diff --git a/frontend/src/app/workspace/component/property-editor/property-editor.component.spec.ts b/frontend/src/app/workspace/component/property-editor/property-editor.component.spec.ts index 67ea6d0e7d9..68e36e5fe8c 100644 --- a/frontend/src/app/workspace/component/property-editor/property-editor.component.spec.ts +++ b/frontend/src/app/workspace/component/property-editor/property-editor.component.spec.ts @@ -81,6 +81,34 @@ describe("PropertyEditorComponent", () => { expect(component).toBeTruthy(); }); + // The Form View mounts this panel with persistPlacement=false. It must not persist the docked + // canvas panel's geometry -- it is not that panel, and writing these keys would overwrite the + // real one's saved size. (The ngOnInit restore is guarded by the same flag; its canvas path is + // exercised by the default fixture above.) + it("does not persist the docked panel geometry when persistPlacement is false", () => { + component.persistPlacement = false; + const setItem = vi.spyOn(Storage.prototype, "setItem"); + + component.ngOnDestroy(); + + expect(setItem).not.toHaveBeenCalledWith("right-panel-width", expect.anything()); + expect(setItem).not.toHaveBeenCalledWith("right-panel-style", expect.anything()); + }); + + // The crash this flag fixes: ngOnInit reads #right-container to restore the docked panel's + // placement, and that element only exists in the canvas layout. The Form View mounts the panel + // with persistPlacement=false, where the element is absent -- reading it there would throw. With + // the flag off, ngOnInit must not go near it. Re-run ngOnInit on the existing instance (no second + // fixture, which would pollute TestBed) with the flag off and assert the lookup never happens. + it("does not read #right-container in ngOnInit when persistPlacement is false", () => { + component.persistPlacement = false; + const getById = vi.spyOn(document, "getElementById"); + + expect(() => component.ngOnInit()).not.toThrow(); + + expect(getById).not.toHaveBeenCalledWith("right-container"); + }); + /** * test if the property editor correctly receives the operator unhighlight stream * and clears all the operator data, and hide the form. @@ -98,6 +126,7 @@ describe("PropertyEditorComponent", () => { expect(component.componentInputs).toEqual({ currentOperatorId: mockScanPredicate.operatorID, exposeChoosing: false, + broadcastEditing: true, }); // unhighlight the operator @@ -148,6 +177,7 @@ describe("PropertyEditorComponent", () => { expect(component.componentInputs).toEqual({ currentOperatorId: mockScanPredicate.operatorID, exposeChoosing: false, + broadcastEditing: true, }); // unhighlight the operator @@ -164,6 +194,7 @@ describe("PropertyEditorComponent", () => { expect(component.componentInputs).toEqual({ currentOperatorId: mockResultPredicate.operatorID, exposeChoosing: false, + broadcastEditing: true, }); }); diff --git a/frontend/src/app/workspace/component/property-editor/property-editor.component.ts b/frontend/src/app/workspace/component/property-editor/property-editor.component.ts index e6006926043..26d66845c89 100644 --- a/frontend/src/app/workspace/component/property-editor/property-editor.component.ts +++ b/frontend/src/app/workspace/component/property-editor/property-editor.component.ts @@ -92,6 +92,24 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges { * Forwarded to the operator frame, which puts a tick box beside each property. */ @Input() exposeChoosing = false; + /** + * Whether this panel owns the docked canvas panel's saved size/position. The Form View mounts + * this same component read-only inside a preview box, where the canvas layout (#right-container) + * does not exist; with `false` it neither restores nor persists that shared placement, so it + * cannot crash on the missing element and cannot overwrite the canvas panel's geometry. Default + * `true` keeps the operator canvas exactly as it was. + */ + @Input() persistPlacement = true; + /** + * Whether opening an operator here broadcasts "currently editing this operator" on the shared + * co-editor channel. On the operator canvas that is right (a co-editor should see who is editing + * what). The Form View opens the panel only to inspect a step read-only, so it mounts with + * `false`: a reader is not editing the graph, and broadcasting would print the reader's own name + * in colour over that operator on everyone else's canvas. Default `true` keeps the canvas as it + * was. Forwarded to the operator frame, which owns the actual writes (the editing broadcast and + * the operator-version sync). + */ + @Input() broadcastEditing = true; /** Set from the toolbar toggle on the operator canvas; the input covers the form view. */ private choosingFromToolbar = false; @@ -137,11 +155,16 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges { } ngOnInit(): void { - const style = localStorage.getItem("right-panel-style"); - if (style) document.getElementById("right-container")!.style.cssText = style; - const translates = document.getElementById("right-container")!.style.transform; - const [xOffset, yOffset, _] = calculateTotalTranslate3d(translates); - this.returnPosition = { x: -xOffset, y: -yOffset }; + // Restoring the docked panel's saved placement reads #right-container, which only exists in the + // canvas layout. The Form View mounts this panel with persistPlacement=false, where that element + // is absent, so skip the restore there (it would throw on the missing element). + if (this.persistPlacement) { + const style = localStorage.getItem("right-panel-style"); + if (style) document.getElementById("right-container")!.style.cssText = style; + const translates = document.getElementById("right-container")!.style.transform; + const [xOffset, yOffset, _] = calculateTotalTranslate3d(translates); + this.returnPosition = { x: -xOffset, y: -yOffset }; + } this.registerHighlightEventsHandler(); // The toolbar's "choose fields" toggle lives in the service so both the canvas toolbar // and this panel see the same state. Re-emit the frame's inputs when it changes, so tick @@ -205,6 +228,11 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges { @HostListener("window:beforeunload") ngOnDestroy(): void { + // The Form View's read-only copy (persistPlacement=false) must not persist geometry: it is not + // the docked canvas panel, so writing these keys would overwrite the real panel's saved size. + if (!this.persistPlacement) { + return; + } localStorage.setItem("right-panel-width", String(this.width)); localStorage.setItem("right-panel-height", String(this.height)); @@ -248,7 +276,11 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges { if (highlightedOperators.length === 1 && highlightLinks.length === 0 && highlightedPorts.length === 0) { this.currentComponent = OperatorPropertyEditFrameComponent; - this.componentInputs = { currentOperatorId: highlightedOperators[0], exposeChoosing: this.choosing }; + this.componentInputs = { + currentOperatorId: highlightedOperators[0], + exposeChoosing: this.choosing, + broadcastEditing: this.broadcastEditing, + }; } else if (highlightedPorts.length === 1 && highlightLinks.length === 0) { this.currentComponent = PortPropertyEditFrameComponent; this.componentInputs = { currentPortID: highlightedPorts[0] }; diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html index 0b268628392..2d19de54554 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html @@ -221,6 +221,40 @@

{{ instructionTitle || "How to use this" }}

+ + + + + +
+ +
diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss index 77185232a69..4490bb50e4d 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss @@ -569,6 +569,90 @@ $shell: #fafafa; } } +/* ---------- inspect a step (read-only property panel over the preview) ---------- */ + +/* Sibling of the panel, pinned to the same corner and above it (see the html note). */ +.panel-close { + position: absolute; + top: 18px; + right: 20px; + z-index: 20; + width: 24px; + height: 24px; + border: 0; + border-radius: 4px; + background: none; + color: rgba(0, 0, 0, 0.45); + cursor: pointer; + display: flex; + align-items: center; + justify-content: center; + + &:hover { + background: #f5f5f5; + color: rgba(0, 0, 0, 0.85); + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: 1px; + } +} + +/* The property panel floats over the preview so it costs no layout space and the workflow keeps the + full width. It is the scroll container (so a long panel is still readable); the property editor + inside is `inert`, which is what makes it read-only for pointer AND keyboard. Its own styling is + left alone -- it should look exactly like the panel on the operator canvas. */ +.panel { + position: absolute; + right: 10px; + top: 10px; + bottom: 10px; + width: 300px; + z-index: 5; + background: #fff; + border: 1px solid $border; + border-radius: 8px; + box-shadow: 0 6px 20px rgba(0, 0, 0, 0.12); + overflow: auto; + display: flex; + + /* Unpin the editor: on the operator canvas it fixes itself to the window edge, which inside this + box would put it off-screen. Here it simply fills the panel. */ + texera-property-editor { + display: block; + flex: 1; + position: static; + overflow: visible; + } + + /* The panel's own minimise/reset controls belong to the docked canvas panel; here the panel is + embedded and sized by this page, so they would only misbehave. */ + ::ng-deep #property-buttons, + ::ng-deep #docked-buttons { + display: none !important; + } + + /* The canvas panel can be dragged and resized; inside this preview it is simply the right-hand + pane -- fixed, full height, dismissed with its close button or by clicking empty canvas. */ + ::ng-deep #right-container, + ::ng-deep #right-container-legacy { + position: static !important; + transform: none !important; + width: 100% !important; + height: 100% !important; + max-height: none !important; + resize: none !important; + box-shadow: none !important; + cursor: default !important; + } + + ::ng-deep nz-resize-handles, + ::ng-deep .nz-resizable-handle { + display: none !important; + } +} + /* ---------- results ---------- */ .results { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index 67bbec74d3c..5ba57ebec71 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -1331,4 +1331,101 @@ describe("WorkflowFormComponent", () => { expect(component.runError).toBe("Run failed: please check your inputs and try again."); }); }); + + describe("inspecting a step read-only", () => { + const withOp = () => { + h.hasOperatorIds.add("op-1"); + h.graphOperators.push({ operatorID: "op-1", operatorType: "Filter" }); + }; + + // Model a highlight the way the real graph does: the stream emits only the newly-highlighted + // ids (the delta), while getCurrentHighlightedOperatorIDs returns the whole selection. So set + // the full selection first, then emit the delta. + const highlight = (full: string[], delta: string[] = full) => { + h.highlightedIds.length = 0; + h.highlightedIds.push(...full); + h.highlightStream.next(delta); + }; + + it("turns highlighting on so a click selects a step", () => { + build(formViewWorkflow).ngOnInit(); + expect(workflowActionService.setHighlightingEnabled).toHaveBeenCalledWith(true); + }); + + it("opens the read-only panel for the clicked step", () => { + build(formViewWorkflow).ngOnInit(); + withOp(); + + highlight(["op-1"]); + + expect(component.selectedOperatorId).toBe("op-1"); + }); + + it("never broadcasts editing itself: silence is delegated to the panel (broadcastEditing=false)", () => { + build(formViewWorkflow).ngOnInit(); + withOp(); + + highlight(["op-1"]); + + // The form component does not touch the co-editor channel at all; the panel is mounted with + // [broadcastEditing]="false", which suppresses the broadcast at the frame (the only writer). + // The frame's suppression is covered in operator-property-edit-frame.component.spec.ts. + expect(h.updateSharedModelAwareness).not.toHaveBeenCalled(); + }); + + it("clears the selection when the clicked step is not on the graph", () => { + build(formViewWorkflow).ngOnInit(); + (component as any).selectedOperatorId = "old"; + + highlight(["ghost"]); + + expect(component.selectedOperatorId).toBeUndefined(); + }); + + it("closes the panel when the canvas clears its highlight", () => { + build(formViewWorkflow).ngOnInit(); + withOp(); + highlight(["op-1"]); + + h.highlightedIds.length = 0; // nothing highlighted any more + h.unhighlightStream.next([]); + + expect(component.selectedOperatorId).toBeUndefined(); + }); + + it("keeps the panel while another step is still highlighted", () => { + build(formViewWorkflow).ngOnInit(); + withOp(); + highlight(["op-1"]); + h.highlightedIds.push("op-2"); // a second step is still highlighted (no new emit) + + h.unhighlightStream.next(["op-1"]); + + expect(component.selectedOperatorId).toBe("op-1"); + }); + + it("dismisses the panel via the close button, dropping the highlight", () => { + build(formViewWorkflow).ngOnInit(); + withOp(); + highlight(["op-1"]); + + component.closeOperatorPanel(); + + expect(h.unhighlightOperators).toHaveBeenCalledWith("op-1"); + expect(component.selectedOperatorId).toBeUndefined(); + }); + + it("ignores a multi-select highlight, closing the panel (no single step to show)", () => { + build(formViewWorkflow).ngOnInit(); + withOp(); + highlight(["op-1"]); + expect(component.selectedOperatorId).toBe("op-1"); + + // Shift-clicking a second step: the stream emits only the new id, but the full selection is + // now two, so the panel closes rather than opening whichever was clicked last. + highlight(["op-1", "op-2"], ["op-2"]); + + expect(component.selectedOperatorId).toBeUndefined(); + }); + }); }); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index d92384f164b..0538eb3c6e7 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -58,6 +58,7 @@ import { WorkflowWebsocketService } from "../../service/workflow-websocket/workf import { ExecutionState } from "../../types/execute-workflow.interface"; import { Point } from "../../types/workflow-common.interface"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; +import { PropertyEditorComponent } from "../property-editor/property-editor.component"; import { ResultTableFrameComponent } from "../result-panel/result-table-frame/result-table-frame.component"; import { VisualizationFrameContentComponent } from "../visualization-panel-content/visualization-frame-content.component"; import { WorkflowEditorComponent } from "../workflow-editor/workflow-editor.component"; @@ -88,8 +89,9 @@ interface RenderedField { * button (a reader's simplified Run/Stop, sharing the canvas's disable conditions), the * computing-unit selector, a run clock and plain-language failure messages. It then shows the * chosen results underneath -- a table, a visualisation, or a compact "no result yet" -- as a - * display filter that follows the canvas's view-result set and never writes it. Opening a step to - * inspect it read-only, and the authoring mode that picks what to show, are later PRs. A view, not + * display filter that follows the canvas's view-result set and never writes it. A reader can also + * click a step on the embedded preview to open its property panel read-only (inert). The authoring + * mode that turns that panel live and picks what to show is a later PR. A view, not * a new object: it opens the same workflow the canvas does. */ @UntilDestroy() @@ -108,6 +110,7 @@ interface RenderedField { NzTooltipModule, UserIconComponent, ComputingUnitSelectionComponent, + PropertyEditorComponent, ResultTableFrameComponent, VisualizationFrameContentComponent, WorkflowEditorComponent, @@ -155,6 +158,10 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { public isWorkflowValid = true; public isWorkflowEmpty = false; + /** The step whose property panel is open for read-only inspection, if any. The panel shows the + * operator's own title, so this id is all the page needs to track. */ + public selectedOperatorId?: string; + /** * Which steps' results to show, as a display filter over the canvas's view-result set: the * author's chosen `resultOperatorIds`, kept to only those an operator still has view-result @@ -235,8 +242,48 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { // Give the result tables a realistic height to page against, so they show a screenful of rows // instead of one. (~7 rows; the card scrolls for the rest.) this.panelResizeService.changePanelSize(900, 560); + // Highlighting is off by default; turning it on is what makes a click on a step select it, + // which is how a reader opens that step's panel to inspect it (and, later, an author to expose). + this.workflowActionService.setHighlightingEnabled(true); this.load(wid); + // Selecting a step on the embedded (read-only) canvas opens its property panel read-only. The + // canvas is not editable, but highlighting still works, so reuse it rather than teach the editor + // a second click mode. + this.workflowActionService + .getJointGraphWrapper() + .getJointOperatorHighlightStream() + .pipe(untilDestroyed(this)) + .subscribe(() => { + // The stream emits only the newly-highlighted ids, not the whole selection, so read the + // current selection to decide -- the same source the property panel uses. Exactly one + // highlighted step opens the panel; a shift-click multi-select opens nothing (the panel + // shows no single operator either), rather than opening whichever step was clicked last. + const selected = this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs(); + if (selected.length === 1) { + this.onOperatorClicked(selected[0]); + } else { + this.clearSelection(); + } + // The panel is mounted with [broadcastEditing]="false", so opening a step here never + // announces "currently editing this operator" on the shared co-editor channel -- a reader + // inspecting a step is not editing the graph, and broadcasting would print the reader's own + // name in colour over that operator on everyone else's canvas. Suppressed at the frame (the + // only place that writes it), not here, so it cannot be re-set after this handler runs. + }); + + // Clicking empty canvas clears the highlight; the panel should go with it. + this.workflowActionService + .getJointGraphWrapper() + .getJointOperatorUnhighlightStream() + .pipe(untilDestroyed(this)) + .subscribe(() => { + if (this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs().length === 0) { + this.clearSelection(); + this.cdr.detectChanges(); + } + }); + // A result changing bumps that operator's version (so its chart frame is rebuilt, not reused), // re-limits what the form shows to the currently-viewed set, and re-fits the visualisations. this.workflowResultService @@ -1016,6 +1063,34 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { return this.rendered.some(r => hasRequiredError(r.form)); } + // --------------------------------------------------------------------------- + // Inspecting a step: its property panel, opened read-only from the preview + // --------------------------------------------------------------------------- + + /** Clicking a step opens the workflow's own property panel for it, read-only. */ + public onOperatorClicked(operatorID: string): void { + const graph = this.workflowActionService.getTexeraGraph(); + if (!graph.hasOperator(operatorID)) { + this.clearSelection(); + return; + } + this.selectedOperatorId = operatorID; + this.cdr.detectChanges(); + } + + /** Dismiss the panel: the selection is what holds it open, so drop the highlight and the selection. */ + public closeOperatorPanel(): void { + const wrapper = this.workflowActionService.getJointGraphWrapper(); + wrapper.unhighlightOperators(...wrapper.getCurrentHighlightedOperatorIDs()); + this.clearSelection(); + this.cdr.detectChanges(); + } + + /** No operator selected: this is what closes the property panel. */ + private clearSelection(): void { + this.selectedOperatorId = undefined; + } + /** Open or close the workflow preview; opening it builds the canvas the first time. */ public toggleWorkflow(): void { this.workflowOpen = !this.workflowOpen; diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts index ede84a90fa1..c2a3916d2aa 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts @@ -18,6 +18,7 @@ */ import { DatePipe } from "@angular/common"; +import { Component, Input } from "@angular/core"; import { FormGroup } from "@angular/forms"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { ActivatedRoute, Router } from "@angular/router"; @@ -58,6 +59,7 @@ import { WorkflowConsoleService } from "../../service/workflow-console/workflow- import { WorkflowWebsocketService } from "../../service/workflow-websocket/workflow-websocket.service"; import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; +import { PropertyEditorComponent } from "../property-editor/property-editor.component"; import { ResultTableFrameComponent } from "../result-panel/result-table-frame/result-table-frame.component"; import { VisualizationFrameContentComponent } from "../visualization-panel-content/visualization-frame-content.component"; import { PanelResizeService } from "../../service/workflow-result/panel-resize/panel-resize.service"; @@ -75,6 +77,19 @@ import { GuiConfigService } from "../../../common/service/gui-config.service"; * name/avatar row, the Canvas switch actually firing, the loading/body swap, and the co-editor * row -- which is the review's evidence of the rendered page in place of a screenshot. */ +// A stand-in for the always-mounted property panel. The real one is heavy -- its ngOnInit +// subscribes to the full JointJS highlight-stream set and the panel service -- and it has its +// own spec. This page only needs the panel present (it lives behind [hidden], not *ngIf, so it +// is mounted from the start to catch the highlight that opens it), so swap in a stub carrying the +// two inputs the template binds and nothing else. The swap is on a child of the page, so the +// page's own template still renders as shipped and stays covered. +@Component({ selector: "texera-property-editor", template: "", standalone: true }) +class MockPropertyEditorComponent { + @Input() exposeChoosing = false; + @Input() persistPlacement = true; + @Input() broadcastEditing = true; +} + describe("WorkflowFormComponent (rendered template)", () => { let fixture: ComponentFixture; let workflow$: Subject; @@ -108,6 +123,14 @@ describe("WorkflowFormComponent (rendered template)", () => { TestBed.overrideComponent(ResultTableFrameComponent, { set: { template: "" } }); TestBed.overrideComponent(VisualizationFrameContentComponent, { set: { template: "" } }); /* eslint-enable no-restricted-syntax */ + // Swap the real property panel (a heavy child with its own spec) for the stub above. Done by + // replacing it in the page's imports rather than blanking its template, because the panel's + // trouble is its ngOnInit -- the highlight-stream and panel-service subscriptions -- which a + // blanked template still runs; a stub component has neither. + TestBed.overrideComponent(WorkflowFormComponent, { + remove: { imports: [PropertyEditorComponent] }, + add: { imports: [MockPropertyEditorComponent] }, + }); await TestBed.configureTestingModule({ // forRoot registers the FormlyConfig the form builder needs: the page imports FormlyModule @@ -136,6 +159,7 @@ describe("WorkflowFormComponent (rendered template)", () => { workflowChanged: () => EMPTY, workflowMetaDataChanged: () => EMPTY, formBindingChanged$: EMPTY, + setHighlightingEnabled: vi.fn(), getTexeraGraph: () => ({ triggerCenterEvent: vi.fn(), hasOperator: () => false, @@ -143,6 +167,13 @@ describe("WorkflowFormComponent (rendered template)", () => { getAllOperators: () => [], getOperatorsToViewResult: () => new Set(), getViewResultOperatorsChangedStream: () => EMPTY, + updateSharedModelAwareness: vi.fn(), + }), + getJointGraphWrapper: () => ({ + getJointOperatorHighlightStream: () => EMPTY, + getJointOperatorUnhighlightStream: () => EMPTY, + getCurrentHighlightedOperatorIDs: () => [], + unhighlightOperators: vi.fn(), }), }, }, diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index e74487327b7..17fc51e33e4 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts @@ -75,6 +75,12 @@ export function setupHarness() { const graphOperators: any[] = []; // Operators with view-result ("the eye") on in the canvas -- the ONLY ones the form may show. const viewResultIds = new Set(); + // Selecting a step on the embedded canvas drives the read-only inspect panel. + const highlightStream = new Subject(); + const unhighlightStream = new Subject(); + const highlightedIds: string[] = []; + const unhighlightOperators = vi.fn(); + const updateSharedModelAwareness = vi.fn(); // Operators that produced a non-empty result -- drives hasNonEmptyResult in the result mock. const anyResultIds = new Set(); // The preview centres the embedded graph once it is built; tests assert this fired. @@ -93,6 +99,7 @@ export function setupHarness() { getWorkflowMetadata: () => ({ name: "scGPT", lastModifiedTime: 1767225600000 }), setWorkflowName: vi.fn(), setWorkflowMetadata: vi.fn(), + setHighlightingEnabled: vi.fn(), getTexeraGraph: () => ({ triggerCenterEvent, hasOperator: (id: string) => hasOperatorIds.has(id), @@ -100,6 +107,13 @@ export function setupHarness() { getAllOperators: () => graphOperators, getOperatorsToViewResult: () => new Set(viewResultIds), getViewResultOperatorsChangedStream: () => viewResultChanged.asObservable(), + updateSharedModelAwareness, + }), + getJointGraphWrapper: () => ({ + getJointOperatorHighlightStream: () => highlightStream.asObservable(), + getJointOperatorUnhighlightStream: () => unhighlightStream.asObservable(), + getCurrentHighlightedOperatorIDs: () => highlightedIds, + unhighlightOperators, }), // Exposing or un-exposing a property announces on this stream; the form re-reads its config. formBindingChanged$: new Subject(), @@ -282,5 +296,10 @@ export function setupHarness() { anyResultIds, snapshotById, triggerCenterEvent, + highlightStream, + unhighlightStream, + highlightedIds, + unhighlightOperators, + updateSharedModelAwareness, }; }