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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
