Skip to content

feat(workflow-form): run the workflow from the form and show the author's instruction - #8440

Merged
mengw15 merged 2 commits into
apache:mainfrom
yangzhang75:formview-pr12
Sep 7, 2026
Merged

mengw15 merged 2 commits into
apache:mainfrom
yangzhang75:formview-pr12

Conversation

@yangzhang75

@yangzhang75 yangzhang75 commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

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.
Screenshot 2026-09-06 at 12 07 57 PM

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.

@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

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.

@codecov-commenter

codecov-commenter commented Sep 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.98658% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.94%. Comparing base (209fc41) to head (2fa059b).

Files with missing lines Patch % Lines
...mponent/workflow-form/workflow-form.component.html 91.42% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8440      +/-   ##
============================================
+ Coverage     93.93%   93.94%   +0.01%     
  Complexity     4824     4824              
============================================
  Files          1209     1209              
  Lines         49471    49618     +147     
  Branches       6028     6056      +28     
============================================
+ Hits          46472    46616     +144     
- Misses         1519     1521       +2     
- Partials       1480     1481       +1     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø) Carriedforward from 209fc41
agent-service 99.32% <ø> (ø) Carriedforward from 209fc41
amber 89.92% <ø> (ø) Carriedforward from 209fc41
computing-unit-managing-service 75.05% <ø> (ø) Carriedforward from 209fc41
config-service 87.12% <ø> (ø) Carriedforward from 209fc41
file-service 83.65% <ø> (ø) Carriedforward from 209fc41
frontend 96.77% <97.98%> (+<0.01%) ⬆️
notebook-migration-service 83.57% <ø> (ø) Carriedforward from 209fc41
pyamber 98.47% <ø> (ø) Carriedforward from 209fc41
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 209fc41

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Refresh events can be permanently lost while typing, validation failures can be misreported, and accessibility issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Extends the workflow Form View with end-to-end execution controls, rendered author instructions, and schema-backed inputs.

Changes:

  • Adds Run/Stop controls, computing-unit selection, timing, and failure messages.
  • Renders collapsible markdown instructions and exposed Formly inputs.
  • Adds unit and rendered-template coverage.
File summaries
File Description
custom-formly-type.ts Identifies canvas-only widgets.
custom-formly-type.spec.ts Tests widget classification.
workflow-form.spec-harness.ts Expands Form View test mocks.
workflow-form.rendered.spec.ts Tests rendered inputs, instructions, and run controls.
workflow-form.component.ts Implements inputs, instructions, and execution behavior.
workflow-form.component.spec.ts Tests the new component logic.
workflow-form.component.scss Styles instructions, inputs, and run controls.
workflow-form.component.html Adds the new Form View interface.
sub-fields.spec.ts Tests nested and repeated-field utilities.
Review details

Suppressed comments (1)

frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:290

  • This drops a binding-change notification permanently whenever an input is focused. After the user leaves the field, a removed, renamed, or reordered binding can therefore remain stale until some unrelated later event. Preserve a pending refresh and apply it on focusout rather than treating the event as a no-op.
      if (this.isTypingInTheForm()) {
        return;
      }
  • Files reviewed: 9/9 changed files
  • Comments generated: 6
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@yangzhang75

Copy link
Copy Markdown
Contributor Author

/request-review: @mengw15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Run controls omit computing-unit permissions and mishandle connection loss during execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Balanced

@mengw15 mengw15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one comment. The rest checks out: the [innerHTML] is a bare binding (Angular's sanitizer applies, no bypass anywhere), matching the dashboard's markdown-description precedent; isConnecting is verbatim the canvas's condition and the disable set matches getRunButtonBehavior; putting Stop ahead of Invalid/Empty is a good divergence — the canvas can strand a running workflow behind a disabled "Invalid Workflow" button, this page can always stop.

…instruction

Add the Run button whose label, icon and disabled state mirror the operator
canvas (Invalid / Empty / Connecting / Connect / Run / Stop, in the same
precedence), the computing-unit selector, a run clock counted off the engine's
own duration event, and a plain-language failure message that collapses opaque
SQL/Java traces to one sentence and blames empty required inputs when that is
the cause. Running is the same execution call the canvas makes.

Show the author's instruction above the inputs as a collapsible, read-only
markdown card, only when there is instruction text. Editing it is part of the
authoring PR; showing the results follows in the next PR.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY

@mengw15 mengw15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for addressing the comments

@mengw15
mengw15 enabled auto-merge September 7, 2026 10:05
@mengw15
mengw15 added this pull request to the merge queue Sep 7, 2026
Merged via the queue into apache:main with commit a268c1c Sep 7, 2026
23 checks passed
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Sep 10, 2026
### What changes were proposed in this PR?

Closes apache#8024. Part of the Form View stack (parent issue apache#8011), stacked
on apache#8440 (PR12).

Shows the chosen results under the workflow that produced them, as a
**display filter over the canvas's view-result set** (settled design):
the form reads which operators the canvas already views and shows those,
and it NEVER writes the canvas's view-result flags, so a normal canvas
user's result-viewing is unaffected.

- One result card per chosen, currently-viewed step that produced a
non-empty result: a paginated table (`texera-result-table-frame`), a
fit-to-card visualisation (`texera-visualization-panel-content`) with
per-result zoom, or the section shows a compact "press Run" / "no result
yet" state. A step that produced nothing collapses out rather than
sitting on a permanent empty card.
- `shownResultIds` = the author's `resultOperatorIds` kept to only those
still in `graph.getOperatorsToViewResult()`; it refreshes on every
result update, so a view-result toggle on the canvas (a co-editor's
included) is reflected without a reload.
- Adds `WorkflowResultService.hasNonEmptyResult` (main only had
`hasAnyResult` = a service exists; this checks the tuple/snapshot count
so an empty result reads as "no result").

Picking which results to show is an authoring action added by a later PR
(apache#8026); opening a step to inspect it read-only is apache#8025. This PR is
display-only.

### Any related issues, documentation, discussions?

Closes apache#8024. Part of the Form View feature (parent issue apache#8011).

### How was this PR tested?

Unit tests (vitest). Direct-construction tests cover the display filter
(shown = chosen intersect currently-viewed; a de-viewed or deleted
operator drops out; never writes view-result), the non-empty gating, the
zoom clamp, `resultKey`/`resultVersion` rebuilding a frame on a new
result, and the result-update subscription.
`workflow-result.service.spec.ts` covers `hasNonEmptyResult` (empty vs
full, paginated vs snapshot). A TestBed test covers the results section
and card markup (the real table/visualisation children are covered by
their own specs; their websocket-backed DI does not run in jsdom). 100%
statement and function coverage on the changed source. `ng test` (146
tests), `ng build gui`, eslint and prettier all pass.

#### Screenshot

The results section under the workflow: a table and a visualisation card
after a run.
<img width="1299" height="492" alt="Screenshot 2026-09-07 at 12 36
40 PM"
src="https://github.com/user-attachments/assets/a05f4e7e-41ec-47b3-97f1-200c74c2b98f"
/>

<!-- 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.

Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: Meng Wang <[email protected]>
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Oct 19, 2026
…not only the pointer (apache#9016)

### What changes were proposed in this PR?

Closes apache#8943. Part of the Form View feature (apache#8011); from the apache#8439
refinements (item 5).

A read-only input card -- what a viewer without write access sees -- is
locked with `pointer-events: none`. That is hit testing only. The bound
formly controls are disabled through `props.disabled`, but the controls
a custom widget draws of its own are not: the HuggingFace audio and
image uploaders each render a plain file input that nothing disables. A
reader could Tab into one inside a locked card, open it with Enter and
choose a file.

For the audio input that is a real write. The widget POSTs the chosen
file to `/api/huggingface/upload-audio`, which authorises any signed-in
user and never asks who may write this workflow, so the file lands in
the server's temp store. Checked on a workflow shared read-only: two
presses of Tab reach the uploader inside the locked card, Enter opens
the file dialog, and the POST comes back 200 with the stored path. The
image uploader is client-side, so there the reader only makes the card
display a picture of their choosing -- a read-only card wires no
write-back and a reader's save is refused, so the operator keeps the
author's value and a run still runs it.

The card now also keeps the keyboard out, through a small directive on
it (`texeraReadOnlyCard`, applied as `[texeraReadOnlyCard]="!canEdit"`
beside the existing `read-only` class):

- Every focusable descendant is taken out of the tab order
(`tabindex="-1"`, its own value remembered) and marked
`aria-disabled="true"`, so there is nothing for Tab to reach and
assistive tech says why rather than going quiet.
- Every control the browser will refuse outright -- button, input,
select, textarea -- is also disabled natively, its previous state
remembered. Out of the tab order is not out of reach: `tabindex="-1"`
ends only sequential navigation and `aria-disabled` only announces, so a
reader's assistive tech could still focus a widget's file input and
activate it, which for the audio one posts the file (Copilot).
- A card that becomes editable gets its controls back, each with the
tabindex and the disabled state it had, and only those this directive
took.
- A control that leaves the card is let go of on the next pass: a widget
redraws its own controls while the card lives on, so a set that only
grew held every detached node for as long as the reader stayed on the
page (Copilot).
- Widgets render their own controls after the card is built and redraw
them as they go, so the locking is re-applied on every change inside the
card: a control added, one whose own tabindex is written back (ng-zorro
writes one when it enables a control), and one a widget enables again
(the audio uploader's `[disabled]="isUploading"`). The observer runs
outside Angular and sets only what is not already set, so one pass
settles it.

`inert` is what the step panel uses for its read-only mount, and the
issue suggested following it; this does not, deliberately. `inert` also
removes the subtree from the accessibility tree, and a reader's card
exists to be read -- the values are what a reader runs the workflow with
-- so that would trade this bug for a worse one. Disabled controls stay
in the accessibility tree and keep being announced. The directive's doc
comment says so, next to the code.

Nothing else changes: the pointer lock, the styles, the card's content
and the author's editing path are untouched, and the directive does
nothing at all while a card is editable.

### Any related issues, documentation, discussions?

Closes apache#8943, which re-files apache#8758 (closed). Part of apache#8439 (item 5) and
the Form View feature apache#8011. Raised by Copilot on apache#8437, apache#8438 and
apache#8440.

Three neighbouring things are deliberately left alone. The widgets' own
controls still do not consult the field's disabled state in their own
handlers -- fixing that is per-widget work (the HuggingFace uploaders,
the dataset picker, the resource value selector), and the card-level
lock covers every widget at once, including ones added later. The upload
endpoint itself authorises on being signed in and nothing else
(`@RolesAllowed("REGULAR","ADMIN")`, no workflow-access check), so it
would still accept a file posted by hand; closing the UI route does not
close that one, and it is worth its own issue. And the step panel's
`inert` mount has the same accessibility-tree problem described above;
changing it is not this issue's scope, and it is worth its own look.

### How was this PR tested?

Unit tests (vitest). The directive's own spec, on a host shaped like a
card (a bound input, a widget's Browse button, a link, a span, a control
with its own `tabindex`): every focusable control in a read-only card is
out of the tab order; each is marked `aria-disabled` while the card is
not inert and its text is still there; an editable card is left alone,
keeping a control's own `tabindex="3"`; a card that becomes editable
gets that `3` back, the untouched control gets no `tabindex`, and the
`aria-disabled` marks go; a control a widget adds after the card was
built is locked; a control whose widget writes its `tabindex` back, or
enables again, is locked again; a click on a locked control dispatches
no activation and the controls report disabled (jsdom does not model a
disabled control refusing focus, so that half is asserted through the
property the browser acts on); the disabled state is given back on
unlock; five rounds of a control being added and removed leave the
tracked set where it started, rather than holding the detached nodes;
the observer is disconnected when the card goes away. The page's
rendered spec wires it through the real template: a read-only viewer's
card, with a button appended the way a widget renders one, has it out of
the tab order and marked, and the card is not inert.

By hand, on two dev builds of the same base commit, one with this change
and one without, signed in as a second account the workflow is shared
read-only with. Before: from the view toggle, two presses of Tab land on
the uploader inside the locked card, Enter opens the file dialog, and
choosing a file POSTs it (200). After: the same two presses land on Run,
the card is skipped, and no request goes out. The mouse reaches nothing
in either build, and the stored workflow is unchanged in both.

Each guard was deletion-checked and turned a named test red (the
directive not applied; the tabindex write; the aria-disabled write; the
native disable; the editable check; the unlock; the original-tabindex
restore; the original-disabled restore; the release of what has left the
card; the childList and both attribute halves of the observer; the
disconnect). eslint, prettier and the production (AOT) build pass; every
statement, function and branch this PR adds is covered, and the full
frontend suite passes.

#### Video

**Before**



https://github.com/user-attachments/assets/cd1b58ff-6cc4-4958-ad78-dd00bb86f14e



**After**


https://github.com/user-attachments/assets/8f524126-d370-48f3-bc70-68497791cf5f




### Was this PR authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Code (Anthropic). Co-authored with Claude,
reviewed line by line by the author before submission.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(workflow-form): run the workflow from the form and show the author's instruction

4 participants