You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "yangzhang75 (via GitHub)" <gi...@apache.org> on 2026/09/06 08:21:33 UTC
[PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
yangzhang75 opened a new pull request, #8440:
URL: https://github.com/apache/texera/pull/8440
### What changes were proposed in this PR?
Closes #8023. Part of the Form View stack (parent issue #8011), stacked on #8438 (PR11) and #8437 (PR10).
The Form View can now render inputs and write them back. This PR makes it usable end to end: it runs the workflow, and shows the author's instruction above the inputs.
- **Run.** A Run button that 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. The unit is chosen in the embedded computing-unit selector. Running is the same execution call the canvas makes, so empty or invalid inputs surface as a real engine error rather than a client-side gate; a run clock counts off the engine's own event, and a failed run collapses an opaque trace to one plain sentence (a required input left empty reads as "please fill in the required fields").
- **Instruction.** The author's optional instruction renders above the inputs as a collapsible, read-only markdown card, shown only when there is text. It is bound through `[innerHTML]` with Angular's default sanitizer, so author markdown cannot inject script. Editing it is part of the authoring PR.
Showing the chosen results is the next PR (#8024).
### Any related issues, documentation, discussions?
Closes #8023. Part of the Form View feature (parent issue #8011).
### How was this PR tested?
Unit tests (vitest). Direct-construction tests cover every run-button precedence branch, `onRun` (kill while running, no-op when disabled, else execute and clear the error), the five failed-run message paths, the run clock and its tick, and the status/validation/connection subscriptions; the instruction is covered for the markdown render, `hasInstruction`, and the toggle. TestBed template tests stand up the instruction card, the run bar, and the run-button click binding. 100% statement and function coverage on the changed source (the remaining uncovered branches are the pre-existing defensive `??`/`||` fallbacks). `ng test` (99 tests), `ng build gui`, eslint and prettier all pass, in both the single-user and collaboration paths.
#### Screenshot
The run bar (Run button + computing-unit selector + clock) and the author's instruction card above the inputs.
<!-- paste the screenshot below this line -->
### Was this PR authored or co-authored using generative AI tooling?
Yes. Co-authored with Claude (Anthropic), reviewed line by line by the author before submission.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8440:
URL: https://github.com/apache/texera/pull/8440#issuecomment-5558038619
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/8440?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `95.86207%` with `12 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 94.04%. Comparing base ([`27f21a9`](https://app.codecov.io/gh/apache/texera/commit/27f21a9ba2d215767aec385a77d82d426deaf77c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4bf0974`](https://app.codecov.io/gh/apache/texera/commit/4bf0974a23e8f397e3f2e30c63639a92f2e9225f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/texera/pull/8440?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...component/workflow-form/workflow-form.component.ts](https://app.codecov.io/gh/apache/texera/pull/8440?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.component.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5jb21wb25lbnQudHM=) | 96.00% | [0 Missing and 8 partials :warning: ](https://app.codecov.io/gh/apache/texera/pull/8440?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...mponent/workflow-form/workflow-form.component.html](https://app.codecov.io/gh/apache/texera/pull/8440?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.component.html&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5jb21wb25lbnQuaHRtbA==) | 94.54% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/texera/pull/8440?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ponent/workflow-form/workflow-form.spec-harness.ts](https://app.codecov.io/gh/apache/texera/pull/8440?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.spec-harness.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5zcGVjLWhhcm5lc3MudHM=) | 97.05% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/texera/pull/8440?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8440 +/- ##
============================================
+ Coverage 94.03% 94.04% +0.01%
Complexity 4820 4820
============================================
Files 1208 1208
Lines 49219 49504 +285
Branches 5982 6038 +56
============================================
+ Hits 46284 46557 +273
- Misses 1464 1466 +2
- Partials 1471 1481 +10
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `99.32% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.90% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.67% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.12% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.91% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.77% <95.86%> (-0.02%)` | :arrow_down: | |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.57% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.47% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/8440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.19% <ø> (ø)` | | Carriedforward from [60a0086](https://app.codecov.io/gh/apache/texera/commit/60a0086449597734a4ec7dd0135b0f0abccdd6dc?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/8440?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8440:
URL: https://github.com/apache/texera/pull/8440#issuecomment-5557999087
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- No candidates found from `git blame` history.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8440:
URL: https://github.com/apache/texera/pull/8440#discussion_r3943471582
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -70,8 +70,125 @@
</div>
<div [hidden]="loading">
- <!-- The workflow, out of the way unless the reader goes looking. The inputs, running and
- results are added on top of this by the following PRs. -->
+ <!-- The author's one piece of guidance, shown as rendered markdown, collapsible, and only when
+ there is instruction text to show. Editing it is part of the authoring PR. -->
+ <section
+ class="card instr"
+ *ngIf="hasInstruction"
+ [class.open]="instructionOpen">
+ <button
+ type="button"
+ class="instr-bar"
+ [attr.aria-expanded]="instructionOpen"
+ (click)="toggleInstruction()">
+ <i
+ nz-icon
+ nzType="info-circle"
+ class="lead"
+ aria-hidden="true"></i>
+ <h2>{{ instructionTitle || "How to use this" }}</h2>
+ <i
+ nz-icon
+ nzType="down"
+ class="chev"
+ aria-hidden="true"></i>
+ </button>
+
+ <div
+ class="instr-body"
+ [hidden]="!instructionOpen">
+ <div
+ class="md"
+ [innerHTML]="instructionPreviewHtml"></div>
+ </div>
+ </section>
+
+ <!-- The inputs an author exposed, each rendered as its operator's own field. -->
+ <div class="pc-section-head">
+ <span class="label">Inputs</span>
+ </div>
+
+ <div
+ class="empty"
+ *ngIf="visibleFields.length === 0">
+ This workflow has no inputs to fill in.
+ </div>
+
+ <div class="params">
+ <section
+ class="card param"
+ [class.read-only]="!canEdit"
+ *ngFor="let r of rendered; trackBy: trackByRendered">
+ <!-- The operator's own field, so a file property gets the real file picker and an
+ attribute property a column dropdown. -->
+ <form
+ [formGroup]="r.form"
+ class="param-form">
+ <formly-form
+ [model]="r.model"
+ [fields]="r.fields"
+ [form]="r.form"></formly-form>
+ </form>
+
+ <!-- The one line of guidance a reader gets, when the author wrote one. The operator
+ schema's own field descriptions are dropped (author notes, not reader guidance), so
+ this is the only help text under an input. -->
+ <p
+ *ngIf="r.resolved.binding.helpText"
+ class="param-help-text">
+ {{ r.resolved.binding.helpText }}
+ </p>
+ </section>
+ </div>
+
+ <!-- Run and the unit it runs on sit together, because one gates the other, but they stay two
+ controls: the selector draws its own bordered box. Adjacency and equal height say they
+ belong to each other. -->
+ <div class="runbar">
+ <div class="run-group">
+ <!-- One button in this page's own style, but its label, icon and disabled-ness are the
+ operator canvas's own (menu.component's getRunButtonBehavior): "Invalid" / "Empty" for a
+ graph that cannot run, "Connecting" while a unit's socket comes up, "Connect" before one
+ is chosen, "Run" once ready, "Stop" while running. Disabled in every state a run cannot
+ start from, so the reader is never sent to press a button that does nothing. -->
+ <button
+ class="run"
+ [class.stop]="isRunning"
+ [class.connecting]="runButtonState.disabled && !isRunning"
+ [disabled]="runButtonState.disabled"
+ (click)="onRun()">
+ <i
+ nz-icon
+ [nzType]="runButtonState.icon"
+ aria-hidden="true"></i>
+ {{ runButtonState.label }}
+ </button>
+ <!-- The page opens its own connection on load, so this control no longer decides whether
+ running works by where it is mounted. -->
+ <div class="run-unit">
+ <texera-computing-unit-selection></texera-computing-unit-selection>
+ </div>
+ <!-- How long this has been going, from the same engine event the operator canvas counts. It
+ appears only once there is something to count, so a form at rest is not carrying a
+ 0:00:00 around. -->
+ <span
+ class="run-clock"
+ *ngIf="executionDuration > 0">
+ {{ executionDuration | date: "H:mm:ss" : "UTC" }}
+ </span>
+ </div>
+ </div>
+ <!-- On its own line: a message that explains or blocks the run should not push the two controls
+ apart, and it is absent most of the time. -->
+ <p
+ class="run-note"
+ [class.err]="runError"
+ *ngIf="runError || isRunning">
+ {{ runError || "Running -- this keeps going if you look away." }}
+ </p>
Review Comment:
The run status and failure text is inserted dynamically but has no live-region semantics, so screen-reader users are not notified when a run fails or begins. Give failures `role="alert"` and the running message `role="status"`.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss:
##########
@@ -183,13 +183,307 @@ $shell: #fafafa;
padding: 40px 0;
}
-/* A section of the page: the workflow preview here, the inputs and results in later PRs. */
+/* A section of the page: the inputs and the workflow preview here, results in later PRs. */
.card {
border: 1px solid $border;
border-radius: 8px;
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 {
+ display: flex;
+ align-items: baseline;
+ gap: 12px;
+ margin-bottom: 12px;
+}
+
+.label {
+ font-size: 12px;
+ font-weight: 600;
+ letter-spacing: 0.08em;
+ text-transform: uppercase;
+ color: $text-2;
+}
+
+.empty {
+ padding: 28px;
+ text-align: center;
+ color: $text-2;
+ border: 1px dashed $border;
+ border-radius: 8px;
+}
+
+.params {
+ display: flex;
+ flex-direction: column;
+ gap: 12px;
+}
+
+.param {
+ padding: 16px 18px;
+
+ /* A read-only viewer sees the values but cannot change them. The bound controls are already
+ disabled through props.disabled; this blocks every other interactive element a custom widget
+ draws of its own -- a file picker's Browse button, an uploader, a picker's dropdown -- which
+ do not consult the form's disabled state. */
+ &.read-only {
+ pointer-events: none;
+ }
Review Comment:
`pointer-events: none` only blocks pointer input. The custom picker buttons do not honor the Formly control's disabled state, so a read-only viewer can still tab to them and activate them with Enter/Space. Disable those descendant actions for keyboard users as well rather than relying on pointer-event suppression.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -70,8 +70,125 @@
</div>
<div [hidden]="loading">
- <!-- The workflow, out of the way unless the reader goes looking. The inputs, running and
- results are added on top of this by the following PRs. -->
+ <!-- The author's one piece of guidance, shown as rendered markdown, collapsible, and only when
+ there is instruction text to show. Editing it is part of the authoring PR. -->
+ <section
Review Comment:
The PR description's Screenshot section still contains only the placeholder. This visible frontend change needs before/after screenshots or a GIF as required by the repository's frontend PR guidance.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +347,376 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ // ---------------------------------------------------------------------------
+ // Inputs: the exposed properties, rendered as their operators' own fields
+ // ---------------------------------------------------------------------------
+
+ /** Whether the cursor is currently inside one of this page's inputs. */
+ private isTypingInTheForm(): boolean {
+ const active = document.activeElement as HTMLElement | null;
+ if (!active || !this.host.nativeElement.contains(active)) {
+ return false;
+ }
+ return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
+ }
+
+ 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();
+ }
+
+ /**
+ * 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.
+ */
+ private buildForm(): void {
+ this.formsRebuilt.next();
+ this.rendered = this.visibleFields
+ .map(field => this.renderField(field))
+ .filter((r): r is RenderedField => r !== undefined);
+ }
+
+ private renderField(resolved: ResolvedField): RenderedField | undefined {
+ const { binding } = resolved;
+ const schema = this.operatorSchemaFor(binding.operatorID);
+ if (!schema) {
+ return undefined;
+ }
+ const operator = this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID);
+ const operatorType = operator?.operatorType;
+ const full = this.formlyJsonschema.toFieldConfig(cloneDeep(schema) as never, {
+ map: (mapped, source) => {
+ // Render the exact custom widget the operator property panel would (file/model/dataset
+ // pickers, image/audio uploaders, ...), shared via customFormlyFieldType so an exposed
+ // property shows its real control instead of degrading to a text box.
+ const customType = customFormlyFieldType({
+ key: mapped.key,
+ operatorType,
+ description: (source as { description?: string })?.description,
+ currentType: mapped.type,
+ });
+ // Canvas-only widgets (code editor, drag-reorder) do not work here; an older workflow may
+ // already carry one, so leave it to formly's default editable control rather than a widget
+ // that cannot function on a form.
+ if (customType && !CANVAS_ONLY_FORMLY_TYPES.has(customType)) {
+ mapped.type = customType;
+ }
+ return mapped;
+ },
+ });
+ const source = (full.fieldGroup ?? []).find(child => child.key === binding.propertyKey);
+ if (!source) {
+ return undefined;
+ }
+
+ const field = cloneDeep(source);
+ // The schema's own title ("Attributes", "Limit", "File") -- the reader's title when unnamed.
+ // Falls back to this, not the lower-camel key ("fileName"), which would read inconsistently.
+ const schemaLabel = (source.props?.label as string) || binding.propertyKey;
+ field.key = binding.id;
+ field.props = {
+ ...(field.props ?? {}),
+ label: binding.displayName || schemaLabel,
+ };
+
+ const form = new FormGroup({});
+ // Seed the model with the operator's other properties as read-only context, not just this
+ // input's own value: some custom widgets read a sibling to decide what to show -- the
+ // HuggingFace model picker reads `task` to load the right models and label the field. Only
+ // this binding's value is ever written back (see below); the context is never persisted, and
+ // it is cloned so a widget that mutates it cannot reach through to the real operator.
+ const model: Record<string, unknown> = {
+ ...cloneDeep(operator?.operatorProperties ?? {}),
+ [binding.id]: cloneDeep(resolved.value),
+ };
+ if (this.canEdit) {
+ form.valueChanges
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), takeUntil(this.formsRebuilt), untilDestroyed(this))
+ .subscribe(() => {
+ // Formly emits the schema's empty default while building the control, before any edit;
+ // writing that back silently wiped the operator's real value (both views edit one
+ // workflow). So only accept a dirtied form, or a value that differs from the operator's
+ // without being emptier (some controls set values without marking dirty).
+ const next = model[binding.id];
+ const current = this.formBindingService.readValue(binding.operatorID, binding.propertyKey);
+ const isEmpty = (v: unknown) => v === undefined || v === null || v === "";
+ const unchanged = JSON.stringify(next ?? null) === JSON.stringify(current ?? null);
+ if (unchanged || (!form.dirty && isEmpty(next) && !isEmpty(current))) {
+ return;
+ }
+ // Write straight onto the operator (the same edit the canvas makes) and refresh this
+ // card's snapshot, which the template reads.
+ this.formBindingService.writeValue(binding, next);
+ this.parameters = this.formBindingService.resolveFields();
+ const refreshed = this.parameters.find(p => p.binding.id === binding.id);
+ const card = this.rendered.find(r => r.resolved.binding.id === binding.id);
+ if (refreshed && card) {
+ card.resolved = refreshed;
+ }
+ this.cdr.detectChanges();
+ });
+ } else {
+ // A read-only viewer sees the author's values and can run with them, but cannot change them.
+ // Disable at the field level, not with form.disable(): formly builds its controls into the
+ // form after this, and a FormGroup disabled while still empty does not disable controls added
+ // later (it re-enables itself), so the input stayed editable. props.disabled is what formly
+ // honours, and it cascades to a nested property's sub-fields. No write-back is wired either.
+ field.props = { ...(field.props ?? {}), disabled: true };
+ }
+
+ this.applyFieldOverrides(field, binding);
+ return { resolved, fields: [field], form, model };
+ }
+
+ /**
+ * The template for one row of a repeated section. formly's `fieldArray` may be the template
+ * object or a function that builds one per row; resolve both so an array property's sub-fields
+ * are reachable (treating the function case as a leaf hid them). @internal, exported for tests.
+ */
+ public static arrayItemOf(node: FormlyFieldConfig): FormlyFieldConfig | undefined {
+ const fa = node.fieldArray;
+ if (!fa) {
+ return undefined;
+ }
+ if (typeof fa !== "function") {
+ return fa;
+ }
+ try {
+ return fa(node);
+ } catch {
+ // A builder that needs more context than we can give it tells us nothing about the row's
+ // shape; better to list no sub-fields than to guess at them.
+ return undefined;
+ }
+ }
+
+ /**
+ * The override path for a child field: the parent path joined with the child's key, but array
+ * indices are dropped so one override entry covers every row of a repeated section. @internal,
+ * exported for tests.
+ */
+ public static childPath(parent: string, key: unknown): string {
+ if (typeof key !== "string" || key === "" || /^\d+$/.test(key)) {
+ return parent;
+ }
+ return parent ? parent + "." + key : key;
+ }
+
+ /**
+ * Walk the field and its sub-fields, dropping the operator schema's own per-field descriptions
+ * (author notes about the operator, not guidance to a form reader) and applying the author's
+ * stored per-sub-field overrides (rename, hide), keyed by field path. A repeated section builds
+ * its row template on demand, so its builder is wrapped to decorate every row formly ever makes.
+ */
+ private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding): void {
+ const walk = (node: FormlyFieldConfig, path: string): void => {
+ // Drop the schema's own description on every field, nested ones included: on this page the
+ // one piece of guidance is the help text the form's author writes, rendered once by the card.
+ node.props = { ...(node.props ?? {}), description: "" };
+ // Apply the author's stored overrides so a reader sees each sub-field renamed and hidden as
+ // set up. The root (path "") carries the binding's own displayName, set in renderField.
+ if (path) {
+ const override = binding.overrides?.[path] ?? {};
+ if (override.displayName) {
+ node.props = { ...(node.props ?? {}), label: override.displayName };
+ }
+ if (override.hidden) {
+ node.hide = true;
+ }
+ }
+ // A repeated section may build its row template on demand, once per row. Decorating the
+ // object it returns is pointless -- the next row gets a fresh one. Wrap the builder instead,
+ // so every row formly ever creates comes out decorated.
+ if (typeof node.fieldArray === "function") {
+ const build = node.fieldArray;
+ node.fieldArray = (f: FormlyFieldConfig) => {
+ const row = build(f);
+ // Walk what is INSIDE each row, never the row container itself: the container carries the
+ // array property's own name, so decorating it as a root (path "") printed the group title
+ // a second time above the rows. Its sub-fields keep their own key paths, the same ones
+ // their overrides are stored under.
+ for (const child of row.fieldGroup ?? []) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ return row;
+ };
+ return;
+ }
+ const arrayItem = WorkflowFormComponent.arrayItemOf(node);
+ const children = node.fieldGroup ?? arrayItem?.fieldGroup ?? [];
+ for (const child of children) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ // A scalar array (e.g. a list of strings) has a row template with no sub-fields of its own;
+ // decorate it directly so its schema description is dropped like every other field's.
+ if (arrayItem && !arrayItem.fieldGroup) {
+ walk(arrayItem, path);
+ }
+ };
+ walk(field, "");
+ }
+
+ private operatorSchemaFor(operatorID: string): object | undefined {
+ const graph = this.workflowActionService.getTexeraGraph();
+ if (!graph.hasOperator(operatorID)) {
+ return undefined;
+ }
+ try {
+ // Prefer the per-instance schema: it carries the upstream column names, so an attribute
+ // picker renders as a dropdown of real columns rather than a text box.
+ return this.dynamicSchemaService.getDynamicSchema(operatorID).jsonSchema;
+ } catch {
+ try {
+ return this.operatorMetadataService.getOperatorSchema(graph.getOperator(operatorID).operatorType).jsonSchema;
+ } catch {
+ return undefined;
+ }
+ }
+ }
+
+ /**
+ * The inputs a reader is offered. Broken bindings (the operator was deleted, or the property key
+ * no longer exists) are left out, since filling one in could not affect a run; the author's view
+ * of them, to repair them, is added by the authoring PR.
+ */
+ public get visibleFields(): ResolvedField[] {
+ return this.parameters.filter(field => !field.brokenReason);
+ }
+
+ public trackByRendered(_: number, rendered: RenderedField): string {
+ 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<void> {
+ this.instructionPreviewHtml = this.instructionBody.trim()
+ ? await Promise.resolve(this.markdownService.parse(this.instructionBody))
+ : "";
+ this.cdr.detectChanges();
+ }
+
+ public toggleInstruction(): void {
+ this.instructionOpen = !this.instructionOpen;
+ }
+
+ // ---------------------------------------------------------------------------
+ // Running. The same call the operator canvas makes, on the same workflow.
+ // ---------------------------------------------------------------------------
+
+ 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;
+ }
+
+ /**
+ * 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 } {
+ 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.isConnecting) {
+ return { label: "Connecting", icon: "loading", disabled: true };
+ }
+ if (this.hasNoComputingUnit) {
+ return { label: "Connect", icon: "plus-circle", 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 {
+ return this.rendered.some(r => r.form.invalid);
+ }
Review Comment:
`FormGroup.invalid` is not specific to empty required fields; it is also true for pattern, range, and custom-validator failures. Any failed engine run while such a field is invalid will incorrectly tell the reader to fill required fields. Inspect descendant controls for an actual `required` error (and an empty value) before selecting this message.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +347,376 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ // ---------------------------------------------------------------------------
+ // Inputs: the exposed properties, rendered as their operators' own fields
+ // ---------------------------------------------------------------------------
+
+ /** Whether the cursor is currently inside one of this page's inputs. */
+ private isTypingInTheForm(): boolean {
+ const active = document.activeElement as HTMLElement | null;
+ if (!active || !this.host.nativeElement.contains(active)) {
+ return false;
+ }
+ return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
+ }
+
+ 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();
+ }
+
+ /**
+ * 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.
+ */
+ private buildForm(): void {
+ this.formsRebuilt.next();
+ this.rendered = this.visibleFields
+ .map(field => this.renderField(field))
+ .filter((r): r is RenderedField => r !== undefined);
+ }
+
+ private renderField(resolved: ResolvedField): RenderedField | undefined {
+ const { binding } = resolved;
+ const schema = this.operatorSchemaFor(binding.operatorID);
+ if (!schema) {
+ return undefined;
+ }
+ const operator = this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID);
+ const operatorType = operator?.operatorType;
+ const full = this.formlyJsonschema.toFieldConfig(cloneDeep(schema) as never, {
+ map: (mapped, source) => {
+ // Render the exact custom widget the operator property panel would (file/model/dataset
+ // pickers, image/audio uploaders, ...), shared via customFormlyFieldType so an exposed
+ // property shows its real control instead of degrading to a text box.
+ const customType = customFormlyFieldType({
+ key: mapped.key,
+ operatorType,
+ description: (source as { description?: string })?.description,
+ currentType: mapped.type,
+ });
+ // Canvas-only widgets (code editor, drag-reorder) do not work here; an older workflow may
+ // already carry one, so leave it to formly's default editable control rather than a widget
+ // that cannot function on a form.
+ if (customType && !CANVAS_ONLY_FORMLY_TYPES.has(customType)) {
+ mapped.type = customType;
+ }
+ return mapped;
+ },
+ });
+ const source = (full.fieldGroup ?? []).find(child => child.key === binding.propertyKey);
+ if (!source) {
+ return undefined;
+ }
+
+ const field = cloneDeep(source);
+ // The schema's own title ("Attributes", "Limit", "File") -- the reader's title when unnamed.
+ // Falls back to this, not the lower-camel key ("fileName"), which would read inconsistently.
+ const schemaLabel = (source.props?.label as string) || binding.propertyKey;
+ field.key = binding.id;
+ field.props = {
+ ...(field.props ?? {}),
+ label: binding.displayName || schemaLabel,
+ };
+
+ const form = new FormGroup({});
+ // Seed the model with the operator's other properties as read-only context, not just this
+ // input's own value: some custom widgets read a sibling to decide what to show -- the
+ // HuggingFace model picker reads `task` to load the right models and label the field. Only
+ // this binding's value is ever written back (see below); the context is never persisted, and
+ // it is cloned so a widget that mutates it cannot reach through to the real operator.
+ const model: Record<string, unknown> = {
+ ...cloneDeep(operator?.operatorProperties ?? {}),
+ [binding.id]: cloneDeep(resolved.value),
+ };
+ if (this.canEdit) {
+ form.valueChanges
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), takeUntil(this.formsRebuilt), untilDestroyed(this))
+ .subscribe(() => {
+ // Formly emits the schema's empty default while building the control, before any edit;
+ // writing that back silently wiped the operator's real value (both views edit one
+ // workflow). So only accept a dirtied form, or a value that differs from the operator's
+ // without being emptier (some controls set values without marking dirty).
+ const next = model[binding.id];
+ const current = this.formBindingService.readValue(binding.operatorID, binding.propertyKey);
+ const isEmpty = (v: unknown) => v === undefined || v === null || v === "";
+ const unchanged = JSON.stringify(next ?? null) === JSON.stringify(current ?? null);
+ if (unchanged || (!form.dirty && isEmpty(next) && !isEmpty(current))) {
+ return;
+ }
+ // Write straight onto the operator (the same edit the canvas makes) and refresh this
+ // card's snapshot, which the template reads.
+ this.formBindingService.writeValue(binding, next);
+ this.parameters = this.formBindingService.resolveFields();
+ const refreshed = this.parameters.find(p => p.binding.id === binding.id);
+ const card = this.rendered.find(r => r.resolved.binding.id === binding.id);
+ if (refreshed && card) {
+ card.resolved = refreshed;
+ }
+ this.cdr.detectChanges();
+ });
+ } else {
+ // A read-only viewer sees the author's values and can run with them, but cannot change them.
+ // Disable at the field level, not with form.disable(): formly builds its controls into the
+ // form after this, and a FormGroup disabled while still empty does not disable controls added
+ // later (it re-enables itself), so the input stayed editable. props.disabled is what formly
+ // honours, and it cascades to a nested property's sub-fields. No write-back is wired either.
+ field.props = { ...(field.props ?? {}), disabled: true };
+ }
+
+ this.applyFieldOverrides(field, binding);
+ return { resolved, fields: [field], form, model };
+ }
+
+ /**
+ * The template for one row of a repeated section. formly's `fieldArray` may be the template
+ * object or a function that builds one per row; resolve both so an array property's sub-fields
+ * are reachable (treating the function case as a leaf hid them). @internal, exported for tests.
+ */
+ public static arrayItemOf(node: FormlyFieldConfig): FormlyFieldConfig | undefined {
+ const fa = node.fieldArray;
+ if (!fa) {
+ return undefined;
+ }
+ if (typeof fa !== "function") {
+ return fa;
+ }
+ try {
+ return fa(node);
+ } catch {
+ // A builder that needs more context than we can give it tells us nothing about the row's
+ // shape; better to list no sub-fields than to guess at them.
+ return undefined;
+ }
+ }
+
+ /**
+ * The override path for a child field: the parent path joined with the child's key, but array
+ * indices are dropped so one override entry covers every row of a repeated section. @internal,
+ * exported for tests.
+ */
+ public static childPath(parent: string, key: unknown): string {
+ if (typeof key !== "string" || key === "" || /^\d+$/.test(key)) {
+ return parent;
+ }
+ return parent ? parent + "." + key : key;
+ }
+
+ /**
+ * Walk the field and its sub-fields, dropping the operator schema's own per-field descriptions
+ * (author notes about the operator, not guidance to a form reader) and applying the author's
+ * stored per-sub-field overrides (rename, hide), keyed by field path. A repeated section builds
+ * its row template on demand, so its builder is wrapped to decorate every row formly ever makes.
+ */
+ private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding): void {
+ const walk = (node: FormlyFieldConfig, path: string): void => {
+ // Drop the schema's own description on every field, nested ones included: on this page the
+ // one piece of guidance is the help text the form's author writes, rendered once by the card.
+ node.props = { ...(node.props ?? {}), description: "" };
+ // Apply the author's stored overrides so a reader sees each sub-field renamed and hidden as
+ // set up. The root (path "") carries the binding's own displayName, set in renderField.
+ if (path) {
+ const override = binding.overrides?.[path] ?? {};
+ if (override.displayName) {
+ node.props = { ...(node.props ?? {}), label: override.displayName };
+ }
+ if (override.hidden) {
+ node.hide = true;
+ }
+ }
+ // A repeated section may build its row template on demand, once per row. Decorating the
+ // object it returns is pointless -- the next row gets a fresh one. Wrap the builder instead,
+ // so every row formly ever creates comes out decorated.
+ if (typeof node.fieldArray === "function") {
+ const build = node.fieldArray;
+ node.fieldArray = (f: FormlyFieldConfig) => {
+ const row = build(f);
+ // Walk what is INSIDE each row, never the row container itself: the container carries the
+ // array property's own name, so decorating it as a root (path "") printed the group title
+ // a second time above the rows. Its sub-fields keep their own key paths, the same ones
+ // their overrides are stored under.
+ for (const child of row.fieldGroup ?? []) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ return row;
+ };
+ return;
+ }
+ const arrayItem = WorkflowFormComponent.arrayItemOf(node);
+ const children = node.fieldGroup ?? arrayItem?.fieldGroup ?? [];
+ for (const child of children) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ // A scalar array (e.g. a list of strings) has a row template with no sub-fields of its own;
+ // decorate it directly so its schema description is dropped like every other field's.
+ if (arrayItem && !arrayItem.fieldGroup) {
+ walk(arrayItem, path);
+ }
+ };
+ walk(field, "");
+ }
+
+ private operatorSchemaFor(operatorID: string): object | undefined {
+ const graph = this.workflowActionService.getTexeraGraph();
+ if (!graph.hasOperator(operatorID)) {
+ return undefined;
+ }
+ try {
+ // Prefer the per-instance schema: it carries the upstream column names, so an attribute
+ // picker renders as a dropdown of real columns rather than a text box.
+ return this.dynamicSchemaService.getDynamicSchema(operatorID).jsonSchema;
+ } catch {
+ try {
+ return this.operatorMetadataService.getOperatorSchema(graph.getOperator(operatorID).operatorType).jsonSchema;
+ } catch {
+ return undefined;
+ }
+ }
+ }
+
+ /**
+ * The inputs a reader is offered. Broken bindings (the operator was deleted, or the property key
+ * no longer exists) are left out, since filling one in could not affect a run; the author's view
+ * of them, to repair them, is added by the authoring PR.
+ */
+ public get visibleFields(): ResolvedField[] {
+ return this.parameters.filter(field => !field.brokenReason);
+ }
+
+ public trackByRendered(_: number, rendered: RenderedField): string {
+ 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<void> {
+ this.instructionPreviewHtml = this.instructionBody.trim()
+ ? await Promise.resolve(this.markdownService.parse(this.instructionBody))
+ : "";
+ this.cdr.detectChanges();
+ }
Review Comment:
Markdown parsing may complete asynchronously, and `readConfig()` can start another parse before the first resolves. The older promise can then overwrite the newer instruction HTML. Capture the body for this render and discard the result if the configured body changed while awaiting it (the existing MarkdownDescriptionComponent uses an equivalent render-sequence guard).
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -121,6 +199,98 @@ 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(() => 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() ?? "");
+ }
+ this.cdr.detectChanges();
+ });
+
+ // 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
+ // on every Canvas<->Form switch) gets the current state at once. Skip it while someone is
+ // typing, so a rebuild does not throw away a half-entered value under the cursor.
+ this.workflowCompilingService
+ .getCompilationStateInfoChangedStream()
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), untilDestroyed(this))
+ .subscribe(() => {
+ if (this.isTypingInTheForm()) {
+ return;
+ }
Review Comment:
A compilation result that arrives while any form control is focused is discarded permanently. This is especially likely after editing an upstream input: compilation finishes while the cursor is still in that input, so dependent attribute fields never receive the new dynamic schema after blur. Queue a pending rebuild and run it on focusout (or otherwise defer until typing ends) instead of returning from the only emission.
This issue also appears on line 288 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8440:
URL: https://github.com/apache/texera/pull/8440#discussion_r3944923000
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -70,8 +70,125 @@
</div>
<div [hidden]="loading">
- <!-- The workflow, out of the way unless the reader goes looking. The inputs, running and
- results are added on top of this by the following PRs. -->
+ <!-- The author's one piece of guidance, shown as rendered markdown, collapsible, and only when
+ there is instruction text to show. Editing it is part of the authoring PR. -->
+ <section
+ class="card instr"
+ *ngIf="hasInstruction"
+ [class.open]="instructionOpen">
+ <button
+ type="button"
+ class="instr-bar"
+ [attr.aria-expanded]="instructionOpen"
+ (click)="toggleInstruction()">
+ <i
+ nz-icon
+ nzType="info-circle"
+ class="lead"
+ aria-hidden="true"></i>
+ <h2>{{ instructionTitle || "How to use this" }}</h2>
+ <i
+ nz-icon
+ nzType="down"
+ class="chev"
+ aria-hidden="true"></i>
+ </button>
+
+ <div
+ class="instr-body"
+ [hidden]="!instructionOpen">
+ <div
+ class="md"
+ [innerHTML]="instructionPreviewHtml"></div>
+ </div>
+ </section>
+
+ <!-- The inputs an author exposed, each rendered as its operator's own field. -->
+ <div class="pc-section-head">
+ <span class="label">Inputs</span>
+ </div>
+
+ <div
+ class="empty"
+ *ngIf="visibleFields.length === 0">
+ This workflow has no inputs to fill in.
+ </div>
+
+ <div class="params">
+ <section
+ class="card param"
+ [class.read-only]="!canEdit"
+ *ngFor="let r of rendered; trackBy: trackByRendered">
+ <!-- The operator's own field, so a file property gets the real file picker and an
+ attribute property a column dropdown. -->
+ <form
+ [formGroup]="r.form"
+ class="param-form">
+ <formly-form
+ [model]="r.model"
+ [fields]="r.fields"
+ [form]="r.form"></formly-form>
+ </form>
+
+ <!-- The one line of guidance a reader gets, when the author wrote one. The operator
+ schema's own field descriptions are dropped (author notes, not reader guidance), so
+ this is the only help text under an input. -->
+ <p
+ *ngIf="r.resolved.binding.helpText"
+ class="param-help-text">
+ {{ r.resolved.binding.helpText }}
+ </p>
+ </section>
+ </div>
+
+ <!-- Run and the unit it runs on sit together, because one gates the other, but they stay two
+ controls: the selector draws its own bordered box. Adjacency and equal height say they
+ belong to each other. -->
+ <div class="runbar">
+ <div class="run-group">
+ <!-- One button in this page's own style, but its label, icon and disabled-ness are the
+ operator canvas's own (menu.component's getRunButtonBehavior): "Invalid" / "Empty" for a
+ graph that cannot run, "Connecting" while a unit's socket comes up, "Connect" before one
+ is chosen, "Run" once ready, "Stop" while running. Disabled in every state a run cannot
+ start from, so the reader is never sent to press a button that does nothing. -->
+ <button
+ class="run"
+ [class.stop]="isRunning"
+ [class.connecting]="runButtonState.disabled && !isRunning"
+ [disabled]="runButtonState.disabled"
+ (click)="onRun()">
+ <i
+ nz-icon
+ [nzType]="runButtonState.icon"
+ aria-hidden="true"></i>
+ {{ runButtonState.label }}
+ </button>
+ <!-- The page opens its own connection on load, so this control no longer decides whether
+ running works by where it is mounted. -->
+ <div class="run-unit">
+ <texera-computing-unit-selection></texera-computing-unit-selection>
+ </div>
+ <!-- How long this has been going, from the same engine event the operator canvas counts. It
+ appears only once there is something to count, so a form at rest is not carrying a
+ 0:00:00 around. -->
+ <span
+ class="run-clock"
+ *ngIf="executionDuration > 0">
+ {{ executionDuration | date: "H:mm:ss" : "UTC" }}
+ </span>
+ </div>
+ </div>
+ <!-- On its own line: a message that explains or blocks the run should not push the two controls
+ apart, and it is absent most of the time. -->
+ <p
+ class="run-note"
+ [class.err]="runError"
+ *ngIf="runError || isRunning">
+ {{ runError || "Running -- this keeps going if you look away." }}
+ </p>
Review Comment:
Fixed: the run note now carries role="alert" on a failure and role="status" while running, so screen readers are notified. Added a test.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +347,376 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ // ---------------------------------------------------------------------------
+ // Inputs: the exposed properties, rendered as their operators' own fields
+ // ---------------------------------------------------------------------------
+
+ /** Whether the cursor is currently inside one of this page's inputs. */
+ private isTypingInTheForm(): boolean {
+ const active = document.activeElement as HTMLElement | null;
+ if (!active || !this.host.nativeElement.contains(active)) {
+ return false;
+ }
+ return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
+ }
+
+ 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();
+ }
+
+ /**
+ * 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.
+ */
+ private buildForm(): void {
+ this.formsRebuilt.next();
+ this.rendered = this.visibleFields
+ .map(field => this.renderField(field))
+ .filter((r): r is RenderedField => r !== undefined);
+ }
+
+ private renderField(resolved: ResolvedField): RenderedField | undefined {
+ const { binding } = resolved;
+ const schema = this.operatorSchemaFor(binding.operatorID);
+ if (!schema) {
+ return undefined;
+ }
+ const operator = this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID);
+ const operatorType = operator?.operatorType;
+ const full = this.formlyJsonschema.toFieldConfig(cloneDeep(schema) as never, {
+ map: (mapped, source) => {
+ // Render the exact custom widget the operator property panel would (file/model/dataset
+ // pickers, image/audio uploaders, ...), shared via customFormlyFieldType so an exposed
+ // property shows its real control instead of degrading to a text box.
+ const customType = customFormlyFieldType({
+ key: mapped.key,
+ operatorType,
+ description: (source as { description?: string })?.description,
+ currentType: mapped.type,
+ });
+ // Canvas-only widgets (code editor, drag-reorder) do not work here; an older workflow may
+ // already carry one, so leave it to formly's default editable control rather than a widget
+ // that cannot function on a form.
+ if (customType && !CANVAS_ONLY_FORMLY_TYPES.has(customType)) {
+ mapped.type = customType;
+ }
+ return mapped;
+ },
+ });
+ const source = (full.fieldGroup ?? []).find(child => child.key === binding.propertyKey);
+ if (!source) {
+ return undefined;
+ }
+
+ const field = cloneDeep(source);
+ // The schema's own title ("Attributes", "Limit", "File") -- the reader's title when unnamed.
+ // Falls back to this, not the lower-camel key ("fileName"), which would read inconsistently.
+ const schemaLabel = (source.props?.label as string) || binding.propertyKey;
+ field.key = binding.id;
+ field.props = {
+ ...(field.props ?? {}),
+ label: binding.displayName || schemaLabel,
+ };
+
+ const form = new FormGroup({});
+ // Seed the model with the operator's other properties as read-only context, not just this
+ // input's own value: some custom widgets read a sibling to decide what to show -- the
+ // HuggingFace model picker reads `task` to load the right models and label the field. Only
+ // this binding's value is ever written back (see below); the context is never persisted, and
+ // it is cloned so a widget that mutates it cannot reach through to the real operator.
+ const model: Record<string, unknown> = {
+ ...cloneDeep(operator?.operatorProperties ?? {}),
+ [binding.id]: cloneDeep(resolved.value),
+ };
+ if (this.canEdit) {
+ form.valueChanges
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), takeUntil(this.formsRebuilt), untilDestroyed(this))
+ .subscribe(() => {
+ // Formly emits the schema's empty default while building the control, before any edit;
+ // writing that back silently wiped the operator's real value (both views edit one
+ // workflow). So only accept a dirtied form, or a value that differs from the operator's
+ // without being emptier (some controls set values without marking dirty).
+ const next = model[binding.id];
+ const current = this.formBindingService.readValue(binding.operatorID, binding.propertyKey);
+ const isEmpty = (v: unknown) => v === undefined || v === null || v === "";
+ const unchanged = JSON.stringify(next ?? null) === JSON.stringify(current ?? null);
+ if (unchanged || (!form.dirty && isEmpty(next) && !isEmpty(current))) {
+ return;
+ }
+ // Write straight onto the operator (the same edit the canvas makes) and refresh this
+ // card's snapshot, which the template reads.
+ this.formBindingService.writeValue(binding, next);
+ this.parameters = this.formBindingService.resolveFields();
+ const refreshed = this.parameters.find(p => p.binding.id === binding.id);
+ const card = this.rendered.find(r => r.resolved.binding.id === binding.id);
+ if (refreshed && card) {
+ card.resolved = refreshed;
+ }
+ this.cdr.detectChanges();
+ });
+ } else {
+ // A read-only viewer sees the author's values and can run with them, but cannot change them.
+ // Disable at the field level, not with form.disable(): formly builds its controls into the
+ // form after this, and a FormGroup disabled while still empty does not disable controls added
+ // later (it re-enables itself), so the input stayed editable. props.disabled is what formly
+ // honours, and it cascades to a nested property's sub-fields. No write-back is wired either.
+ field.props = { ...(field.props ?? {}), disabled: true };
+ }
+
+ this.applyFieldOverrides(field, binding);
+ return { resolved, fields: [field], form, model };
+ }
+
+ /**
+ * The template for one row of a repeated section. formly's `fieldArray` may be the template
+ * object or a function that builds one per row; resolve both so an array property's sub-fields
+ * are reachable (treating the function case as a leaf hid them). @internal, exported for tests.
+ */
+ public static arrayItemOf(node: FormlyFieldConfig): FormlyFieldConfig | undefined {
+ const fa = node.fieldArray;
+ if (!fa) {
+ return undefined;
+ }
+ if (typeof fa !== "function") {
+ return fa;
+ }
+ try {
+ return fa(node);
+ } catch {
+ // A builder that needs more context than we can give it tells us nothing about the row's
+ // shape; better to list no sub-fields than to guess at them.
+ return undefined;
+ }
+ }
+
+ /**
+ * The override path for a child field: the parent path joined with the child's key, but array
+ * indices are dropped so one override entry covers every row of a repeated section. @internal,
+ * exported for tests.
+ */
+ public static childPath(parent: string, key: unknown): string {
+ if (typeof key !== "string" || key === "" || /^\d+$/.test(key)) {
+ return parent;
+ }
+ return parent ? parent + "." + key : key;
+ }
+
+ /**
+ * Walk the field and its sub-fields, dropping the operator schema's own per-field descriptions
+ * (author notes about the operator, not guidance to a form reader) and applying the author's
+ * stored per-sub-field overrides (rename, hide), keyed by field path. A repeated section builds
+ * its row template on demand, so its builder is wrapped to decorate every row formly ever makes.
+ */
+ private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding): void {
+ const walk = (node: FormlyFieldConfig, path: string): void => {
+ // Drop the schema's own description on every field, nested ones included: on this page the
+ // one piece of guidance is the help text the form's author writes, rendered once by the card.
+ node.props = { ...(node.props ?? {}), description: "" };
+ // Apply the author's stored overrides so a reader sees each sub-field renamed and hidden as
+ // set up. The root (path "") carries the binding's own displayName, set in renderField.
+ if (path) {
+ const override = binding.overrides?.[path] ?? {};
+ if (override.displayName) {
+ node.props = { ...(node.props ?? {}), label: override.displayName };
+ }
+ if (override.hidden) {
+ node.hide = true;
+ }
+ }
+ // A repeated section may build its row template on demand, once per row. Decorating the
+ // object it returns is pointless -- the next row gets a fresh one. Wrap the builder instead,
+ // so every row formly ever creates comes out decorated.
+ if (typeof node.fieldArray === "function") {
+ const build = node.fieldArray;
+ node.fieldArray = (f: FormlyFieldConfig) => {
+ const row = build(f);
+ // Walk what is INSIDE each row, never the row container itself: the container carries the
+ // array property's own name, so decorating it as a root (path "") printed the group title
+ // a second time above the rows. Its sub-fields keep their own key paths, the same ones
+ // their overrides are stored under.
+ for (const child of row.fieldGroup ?? []) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ return row;
+ };
+ return;
+ }
+ const arrayItem = WorkflowFormComponent.arrayItemOf(node);
+ const children = node.fieldGroup ?? arrayItem?.fieldGroup ?? [];
+ for (const child of children) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ // A scalar array (e.g. a list of strings) has a row template with no sub-fields of its own;
+ // decorate it directly so its schema description is dropped like every other field's.
+ if (arrayItem && !arrayItem.fieldGroup) {
+ walk(arrayItem, path);
+ }
+ };
+ walk(field, "");
+ }
+
+ private operatorSchemaFor(operatorID: string): object | undefined {
+ const graph = this.workflowActionService.getTexeraGraph();
+ if (!graph.hasOperator(operatorID)) {
+ return undefined;
+ }
+ try {
+ // Prefer the per-instance schema: it carries the upstream column names, so an attribute
+ // picker renders as a dropdown of real columns rather than a text box.
+ return this.dynamicSchemaService.getDynamicSchema(operatorID).jsonSchema;
+ } catch {
+ try {
+ return this.operatorMetadataService.getOperatorSchema(graph.getOperator(operatorID).operatorType).jsonSchema;
+ } catch {
+ return undefined;
+ }
+ }
+ }
+
+ /**
+ * The inputs a reader is offered. Broken bindings (the operator was deleted, or the property key
+ * no longer exists) are left out, since filling one in could not affect a run; the author's view
+ * of them, to repair them, is added by the authoring PR.
+ */
+ public get visibleFields(): ResolvedField[] {
+ return this.parameters.filter(field => !field.brokenReason);
+ }
+
+ public trackByRendered(_: number, rendered: RenderedField): string {
+ 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<void> {
+ this.instructionPreviewHtml = this.instructionBody.trim()
+ ? await Promise.resolve(this.markdownService.parse(this.instructionBody))
+ : "";
+ this.cdr.detectChanges();
+ }
Review Comment:
Fixed: renderInstruction now captures the body it is rendering and discards the result if the configured body changed while parsing, so a stale parse can no longer overwrite a newer one. Added a test.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +347,376 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ // ---------------------------------------------------------------------------
+ // Inputs: the exposed properties, rendered as their operators' own fields
+ // ---------------------------------------------------------------------------
+
+ /** Whether the cursor is currently inside one of this page's inputs. */
+ private isTypingInTheForm(): boolean {
+ const active = document.activeElement as HTMLElement | null;
+ if (!active || !this.host.nativeElement.contains(active)) {
+ return false;
+ }
+ return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
+ }
+
+ 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();
+ }
+
+ /**
+ * 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.
+ */
+ private buildForm(): void {
+ this.formsRebuilt.next();
+ this.rendered = this.visibleFields
+ .map(field => this.renderField(field))
+ .filter((r): r is RenderedField => r !== undefined);
+ }
+
+ private renderField(resolved: ResolvedField): RenderedField | undefined {
+ const { binding } = resolved;
+ const schema = this.operatorSchemaFor(binding.operatorID);
+ if (!schema) {
+ return undefined;
+ }
+ const operator = this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID);
+ const operatorType = operator?.operatorType;
+ const full = this.formlyJsonschema.toFieldConfig(cloneDeep(schema) as never, {
+ map: (mapped, source) => {
+ // Render the exact custom widget the operator property panel would (file/model/dataset
+ // pickers, image/audio uploaders, ...), shared via customFormlyFieldType so an exposed
+ // property shows its real control instead of degrading to a text box.
+ const customType = customFormlyFieldType({
+ key: mapped.key,
+ operatorType,
+ description: (source as { description?: string })?.description,
+ currentType: mapped.type,
+ });
+ // Canvas-only widgets (code editor, drag-reorder) do not work here; an older workflow may
+ // already carry one, so leave it to formly's default editable control rather than a widget
+ // that cannot function on a form.
+ if (customType && !CANVAS_ONLY_FORMLY_TYPES.has(customType)) {
+ mapped.type = customType;
+ }
+ return mapped;
+ },
+ });
+ const source = (full.fieldGroup ?? []).find(child => child.key === binding.propertyKey);
+ if (!source) {
+ return undefined;
+ }
+
+ const field = cloneDeep(source);
+ // The schema's own title ("Attributes", "Limit", "File") -- the reader's title when unnamed.
+ // Falls back to this, not the lower-camel key ("fileName"), which would read inconsistently.
+ const schemaLabel = (source.props?.label as string) || binding.propertyKey;
+ field.key = binding.id;
+ field.props = {
+ ...(field.props ?? {}),
+ label: binding.displayName || schemaLabel,
+ };
+
+ const form = new FormGroup({});
+ // Seed the model with the operator's other properties as read-only context, not just this
+ // input's own value: some custom widgets read a sibling to decide what to show -- the
+ // HuggingFace model picker reads `task` to load the right models and label the field. Only
+ // this binding's value is ever written back (see below); the context is never persisted, and
+ // it is cloned so a widget that mutates it cannot reach through to the real operator.
+ const model: Record<string, unknown> = {
+ ...cloneDeep(operator?.operatorProperties ?? {}),
+ [binding.id]: cloneDeep(resolved.value),
+ };
+ if (this.canEdit) {
+ form.valueChanges
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), takeUntil(this.formsRebuilt), untilDestroyed(this))
+ .subscribe(() => {
+ // Formly emits the schema's empty default while building the control, before any edit;
+ // writing that back silently wiped the operator's real value (both views edit one
+ // workflow). So only accept a dirtied form, or a value that differs from the operator's
+ // without being emptier (some controls set values without marking dirty).
+ const next = model[binding.id];
+ const current = this.formBindingService.readValue(binding.operatorID, binding.propertyKey);
+ const isEmpty = (v: unknown) => v === undefined || v === null || v === "";
+ const unchanged = JSON.stringify(next ?? null) === JSON.stringify(current ?? null);
+ if (unchanged || (!form.dirty && isEmpty(next) && !isEmpty(current))) {
+ return;
+ }
+ // Write straight onto the operator (the same edit the canvas makes) and refresh this
+ // card's snapshot, which the template reads.
+ this.formBindingService.writeValue(binding, next);
+ this.parameters = this.formBindingService.resolveFields();
+ const refreshed = this.parameters.find(p => p.binding.id === binding.id);
+ const card = this.rendered.find(r => r.resolved.binding.id === binding.id);
+ if (refreshed && card) {
+ card.resolved = refreshed;
+ }
+ this.cdr.detectChanges();
+ });
+ } else {
+ // A read-only viewer sees the author's values and can run with them, but cannot change them.
+ // Disable at the field level, not with form.disable(): formly builds its controls into the
+ // form after this, and a FormGroup disabled while still empty does not disable controls added
+ // later (it re-enables itself), so the input stayed editable. props.disabled is what formly
+ // honours, and it cascades to a nested property's sub-fields. No write-back is wired either.
+ field.props = { ...(field.props ?? {}), disabled: true };
+ }
+
+ this.applyFieldOverrides(field, binding);
+ return { resolved, fields: [field], form, model };
+ }
+
+ /**
+ * The template for one row of a repeated section. formly's `fieldArray` may be the template
+ * object or a function that builds one per row; resolve both so an array property's sub-fields
+ * are reachable (treating the function case as a leaf hid them). @internal, exported for tests.
+ */
+ public static arrayItemOf(node: FormlyFieldConfig): FormlyFieldConfig | undefined {
+ const fa = node.fieldArray;
+ if (!fa) {
+ return undefined;
+ }
+ if (typeof fa !== "function") {
+ return fa;
+ }
+ try {
+ return fa(node);
+ } catch {
+ // A builder that needs more context than we can give it tells us nothing about the row's
+ // shape; better to list no sub-fields than to guess at them.
+ return undefined;
+ }
+ }
+
+ /**
+ * The override path for a child field: the parent path joined with the child's key, but array
+ * indices are dropped so one override entry covers every row of a repeated section. @internal,
+ * exported for tests.
+ */
+ public static childPath(parent: string, key: unknown): string {
+ if (typeof key !== "string" || key === "" || /^\d+$/.test(key)) {
+ return parent;
+ }
+ return parent ? parent + "." + key : key;
+ }
+
+ /**
+ * Walk the field and its sub-fields, dropping the operator schema's own per-field descriptions
+ * (author notes about the operator, not guidance to a form reader) and applying the author's
+ * stored per-sub-field overrides (rename, hide), keyed by field path. A repeated section builds
+ * its row template on demand, so its builder is wrapped to decorate every row formly ever makes.
+ */
+ private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding): void {
+ const walk = (node: FormlyFieldConfig, path: string): void => {
+ // Drop the schema's own description on every field, nested ones included: on this page the
+ // one piece of guidance is the help text the form's author writes, rendered once by the card.
+ node.props = { ...(node.props ?? {}), description: "" };
+ // Apply the author's stored overrides so a reader sees each sub-field renamed and hidden as
+ // set up. The root (path "") carries the binding's own displayName, set in renderField.
+ if (path) {
+ const override = binding.overrides?.[path] ?? {};
+ if (override.displayName) {
+ node.props = { ...(node.props ?? {}), label: override.displayName };
+ }
+ if (override.hidden) {
+ node.hide = true;
+ }
+ }
+ // A repeated section may build its row template on demand, once per row. Decorating the
+ // object it returns is pointless -- the next row gets a fresh one. Wrap the builder instead,
+ // so every row formly ever creates comes out decorated.
+ if (typeof node.fieldArray === "function") {
+ const build = node.fieldArray;
+ node.fieldArray = (f: FormlyFieldConfig) => {
+ const row = build(f);
+ // Walk what is INSIDE each row, never the row container itself: the container carries the
+ // array property's own name, so decorating it as a root (path "") printed the group title
+ // a second time above the rows. Its sub-fields keep their own key paths, the same ones
+ // their overrides are stored under.
+ for (const child of row.fieldGroup ?? []) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ return row;
+ };
+ return;
+ }
+ const arrayItem = WorkflowFormComponent.arrayItemOf(node);
+ const children = node.fieldGroup ?? arrayItem?.fieldGroup ?? [];
+ for (const child of children) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
+ // A scalar array (e.g. a list of strings) has a row template with no sub-fields of its own;
+ // decorate it directly so its schema description is dropped like every other field's.
+ if (arrayItem && !arrayItem.fieldGroup) {
+ walk(arrayItem, path);
+ }
+ };
+ walk(field, "");
+ }
+
+ private operatorSchemaFor(operatorID: string): object | undefined {
+ const graph = this.workflowActionService.getTexeraGraph();
+ if (!graph.hasOperator(operatorID)) {
+ return undefined;
+ }
+ try {
+ // Prefer the per-instance schema: it carries the upstream column names, so an attribute
+ // picker renders as a dropdown of real columns rather than a text box.
+ return this.dynamicSchemaService.getDynamicSchema(operatorID).jsonSchema;
+ } catch {
+ try {
+ return this.operatorMetadataService.getOperatorSchema(graph.getOperator(operatorID).operatorType).jsonSchema;
+ } catch {
+ return undefined;
+ }
+ }
+ }
+
+ /**
+ * The inputs a reader is offered. Broken bindings (the operator was deleted, or the property key
+ * no longer exists) are left out, since filling one in could not affect a run; the author's view
+ * of them, to repair them, is added by the authoring PR.
+ */
+ public get visibleFields(): ResolvedField[] {
+ return this.parameters.filter(field => !field.brokenReason);
+ }
+
+ public trackByRendered(_: number, rendered: RenderedField): string {
+ 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<void> {
+ this.instructionPreviewHtml = this.instructionBody.trim()
+ ? await Promise.resolve(this.markdownService.parse(this.instructionBody))
+ : "";
+ this.cdr.detectChanges();
+ }
+
+ public toggleInstruction(): void {
+ this.instructionOpen = !this.instructionOpen;
+ }
+
+ // ---------------------------------------------------------------------------
+ // Running. The same call the operator canvas makes, on the same workflow.
+ // ---------------------------------------------------------------------------
+
+ 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;
+ }
+
+ /**
+ * 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 } {
+ 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.isConnecting) {
+ return { label: "Connecting", icon: "loading", disabled: true };
+ }
+ if (this.hasNoComputingUnit) {
+ return { label: "Connect", icon: "plus-circle", 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 {
+ return this.rendered.some(r => r.form.invalid);
+ }
Review Comment:
Fixed: hasEmptyRequiredInputs now walks the control tree for an actual `required` error rather than any invalid control, so a pattern/range failure yields the engine message instead of "fill in the required fields". Added tests for the required, nested-array, and non-required cases.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss:
##########
@@ -183,13 +183,307 @@ $shell: #fafafa;
padding: 40px 0;
}
-/* A section of the page: the workflow preview here, the inputs and results in later PRs. */
+/* A section of the page: the inputs and the workflow preview here, results in later PRs. */
.card {
border: 1px solid $border;
border-radius: 8px;
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 {
+ display: flex;
+ align-items: baseline;
+ gap: 12px;
+ margin-bottom: 12px;
+}
+
+.label {
+ font-size: 12px;
+ font-weight: 600;
+ letter-spacing: 0.08em;
+ text-transform: uppercase;
+ color: $text-2;
+}
+
+.empty {
+ padding: 28px;
+ text-align: center;
+ color: $text-2;
+ border: 1px dashed $border;
+ border-radius: 8px;
+}
+
+.params {
+ display: flex;
+ flex-direction: column;
+ gap: 12px;
+}
+
+.param {
+ padding: 16px 18px;
+
+ /* A read-only viewer sees the values but cannot change them. The bound controls are already
+ disabled through props.disabled; this blocks every other interactive element a custom widget
+ draws of its own -- a file picker's Browse button, an uploader, a picker's dropdown -- which
+ do not consult the form's disabled state. */
+ &.read-only {
+ pointer-events: none;
+ }
Review Comment:
Same as the read-only thread on the render PR -- pointer-events:none does not block keyboard. Tracked in #8439 (item 5): upgrade the read-only lock to `inert`.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -121,6 +199,98 @@ 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(() => 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() ?? "");
+ }
+ this.cdr.detectChanges();
+ });
+
+ // 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
+ // on every Canvas<->Form switch) gets the current state at once. Skip it while someone is
+ // typing, so a rebuild does not throw away a half-entered value under the cursor.
+ this.workflowCompilingService
+ .getCompilationStateInfoChangedStream()
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), untilDestroyed(this))
+ .subscribe(() => {
+ if (this.isTypingInTheForm()) {
+ return;
+ }
Review Comment:
Tracked in #8439 (item 4): queue a pending rebuild and flush it on focusout instead of dropping the emission. Deferred out of this slice; today the next compilation refreshes it.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -70,8 +70,125 @@
</div>
<div [hidden]="loading">
- <!-- The workflow, out of the way unless the reader goes looking. The inputs, running and
- results are added on top of this by the following PRs. -->
+ <!-- The author's one piece of guidance, shown as rendered markdown, collapsible, and only when
+ there is instruction text to show. Editing it is part of the authoring PR. -->
+ <section
Review Comment:
The screenshot needs a Form-View-flag-on instance (the flag is off by default), so it will be added to the description before this leaves draft.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on PR #8440:
URL: https://github.com/apache/texera/pull/8440#issuecomment-5563604360
/request-review: @mengw15
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 merged PR #8440:
URL: https://github.com/apache/texera/pull/8440
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8440:
URL: https://github.com/apache/texera/pull/8440#discussion_r3947713123
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -164,6 +200,70 @@ 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(() => this.cdr.markForCheck());
Review Comment:
The selected unit is discarded here, so the form never applies the computing-unit permission gate. A shared unit with `READ` or `NONE` privilege still reaches the enabled `Run` state and sends an execution request, whereas the canvas explicitly disables execution unless `selectedComputingUnit.accessPrivilege === Privilege.WRITE` (`menu.component.html:430`); this also contradicts this component's statement at lines 896-898 that execution is gated on computing-unit access. Retain the selected unit and include its write privilege in the run-button disabled state.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -511,6 +616,154 @@ 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<void> {
+ // 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;
+ }
+
+ /**
+ * 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 } {
+ 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.isConnecting) {
+ return { label: "Connecting", icon: "loading", disabled: true };
+ }
+ if (this.hasNoComputingUnit) {
+ return { label: "Connect", icon: "plus-circle", disabled: true };
+ }
+ return { label: "Run", icon: "caret-right", disabled: false };
+ }
Review Comment:
`isRunning` is checked before `isConnecting`, so if the workflow socket drops during an active run this returns an enabled Stop button. Clicking it calls `killWorkflow()`, whose request is sent through the disconnected socket and is not delivered; the canvas disables its kill/run controls in this state. Check the connection state first so the button remains disabled as `Connecting` until the socket reconnects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8440:
URL: https://github.com/apache/texera/pull/8440#discussion_r3948353181
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -164,6 +200,70 @@ 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(() => this.cdr.markForCheck());
Review Comment:
Fixed. The form now keeps the selected unit and gates Run on write access to it: runButtonState returns a disabled "No access" when the chosen unit's accessPrivilege is not WRITE, matching the canvas (menu.component.html:430). A READ/NONE-shared unit now reaches a disabled button instead of sending an execution request, so the save() comment about execution being gated on computing-unit access is accurate. Added tests (No access for a READ unit, plus the wiring that stores the selected unit).
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -511,6 +616,154 @@ 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<void> {
+ // 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;
+ }
+
+ /**
+ * 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 } {
+ 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.isConnecting) {
+ return { label: "Connecting", icon: "loading", disabled: true };
+ }
+ if (this.hasNoComputingUnit) {
+ return { label: "Connect", icon: "plus-circle", disabled: true };
+ }
+ return { label: "Run", icon: "caret-right", disabled: false };
+ }
Review Comment:
Fixed. runButtonState now checks the connecting/disconnected state BEFORE isRunning, so if the socket drops during a run the button stays a disabled "Connecting" rather than an enabled Stop that would send killWorkflow() through a dead socket (matching the canvas, which disables its controls in this state). Added a test for a mid-run socket drop.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -164,6 +200,70 @@ 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(() => 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() ?? "");
+ }
+ this.cdr.detectChanges();
Review Comment:
Agreed, and fixed: the execution-state handler now uses markForCheck() instead of detectChanges(). As you say, this is the one subscription the page cannot afford to lose -- a synchronous pass thrown out of by an unrelated NG0100 would have killed it and frozen the Run button on a stale state. Thanks for catching it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): run the workflow from the form and show the author's instruction [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #8440:
URL: https://github.com/apache/texera/pull/8440#discussion_r3947718467
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -164,6 +200,70 @@ 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(() => 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() ?? "");
+ }
+ this.cdr.detectChanges();
Review Comment:
The rationale above (markForCheck, "a synchronous pass can be thrown out of by an unrelated component's NG0100, killing the subscription") applies most of all to this subscription — if this `detectChanges()` throws, the execution-state stream dies and the Run button is stuck on a stale state with no error surfaced, which is the one subscription this page cannot afford to lose. Worth using `markForCheck()` here too, unless there is a reason this one needs the synchronous pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org