feat(gui): wire the Form View entry points - #8456
Conversation
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8456 +/- ##
============================================
+ Coverage 95.88% 95.90% +0.02%
Complexity 459 459
============================================
Files 588 589 +1
Lines 33802 33953 +151
Branches 4217 4246 +29
============================================
+ Hits 32411 32563 +152
Misses 813 813
+ Partials 578 577 -1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 363 | 0.222 | 28,281/34,499/34,499 us | 🔴 +12.5% / 🔴 +130.3% |
| 🔴 | bs=100 sw=10 sl=64 | 816 | 0.498 | 119,931/141,515/141,515 us | 🔴 +8.8% / 🔴 +37.8% |
| ⚪ | bs=1000 sw=10 sl=64 | 907 | 0.554 | 1,099,779/1,225,814/1,225,814 us | ⚪ within ±5% / 🔴 +25.1% |
Baseline details
Latest main 7190a81 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 363 tuples/sec | 377 tuples/sec | 807.77 tuples/sec | -3.7% | -55.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.222 MB/s | 0.23 MB/s | 0.493 MB/s | -3.5% | -55.0% |
| bs=10 sw=10 sl=64 | p50 | 28,281 us | 25,134 us | 12,280 us | +12.5% | +130.3% |
| bs=10 sw=10 sl=64 | p95 | 34,499 us | 37,873 us | 15,066 us | -8.9% | +129.0% |
| bs=10 sw=10 sl=64 | p99 | 34,499 us | 37,873 us | 19,166 us | -8.9% | +80.0% |
| bs=100 sw=10 sl=64 | throughput | 816 tuples/sec | 829 tuples/sec | 1,054 tuples/sec | -1.6% | -22.6% |
| bs=100 sw=10 sl=64 | MB/s | 0.498 MB/s | 0.506 MB/s | 0.643 MB/s | -1.6% | -22.6% |
| bs=100 sw=10 sl=64 | p50 | 119,931 us | 120,048 us | 96,023 us | -0.1% | +24.9% |
| bs=100 sw=10 sl=64 | p95 | 141,515 us | 130,031 us | 102,660 us | +8.8% | +37.8% |
| bs=100 sw=10 sl=64 | p99 | 141,515 us | 130,031 us | 114,950 us | +8.8% | +23.1% |
| bs=1000 sw=10 sl=64 | throughput | 907 tuples/sec | 924 tuples/sec | 1,086 tuples/sec | -1.8% | -16.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.554 MB/s | 0.564 MB/s | 0.663 MB/s | -1.8% | -16.5% |
| bs=1000 sw=10 sl=64 | p50 | 1,099,779 us | 1,073,687 us | 940,412 us | +2.4% | +16.9% |
| bs=1000 sw=10 sl=64 | p95 | 1,225,814 us | 1,179,829 us | 979,930 us | +3.9% | +25.1% |
| bs=1000 sw=10 sl=64 | p99 | 1,225,814 us | 1,179,829 us | 1,015,625 us | +3.9% | +20.7% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,550.71,200,128000,363,0.222,28281.45,34498.61,34498.61
1,100,10,64,20,2450.35,2000,1280000,816,0.498,119930.80,141514.76,141514.76
2,1000,10,64,20,22052.80,20000,12800000,907,0.554,1099779.27,1225813.77,1225813.7772bb5b2 to
3fb3a3f
Compare
3fb3a3f to
b149959
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Read-only inspection can mutate shared workflow state, while several entry-point and persistence paths remain incomplete or unreliable.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Completes the Form View rollout by adding authoring, navigation entry points, default-view persistence, and computing-unit recall.
Changes:
- Adds in-place Form View authoring and step inspection.
- Adds dashboard/menu navigation and default-view import/export.
- Enables Form View by default and remembers computing-unit selection.
File summaries
| File | Description |
|---|---|
form-binding.service.ts |
Updates result-selection documentation. |
workflow-form.spec-harness.ts |
Expands authoring and selection mocks. |
workflow-form.rendered.spec.ts |
Stubs the embedded property editor. |
workflow-form.component.ts |
Adds inspection and authoring behavior. |
workflow-form.component.spec.ts |
Tests inspection and authoring. |
workflow-form.component.scss |
Styles inspection and authoring UI. |
workflow-form.component.html |
Renders authoring controls and property panel. |
property-editor.component.ts |
Adds placement and broadcast controls. |
property-editor.component.spec.ts |
Tests new property-editor controls. |
operator-property-edit-frame.component.ts |
Suppresses selected shared-model writes. |
operator-property-edit-frame.component.spec.ts |
Tests broadcast suppression. |
computing-unit-selection.component.ts |
Persists selections per workflow. |
computing-unit-selection.component.spec.ts |
Tests computing-unit recall. |
menu.component.ts |
Adds Canvas-to-Form navigation. |
menu.component.spec.ts |
Tests persistence before switching. |
menu.component.scss |
Styles the view switch. |
menu.component.html |
Renders the view switch. |
download.service.ts |
Exports the default view. |
download.service.spec.ts |
Tests exported metadata. |
user-workflow.component.ts |
Imports the default view. |
user-workflow.component.spec.ts |
Tests legacy and new imports. |
list-item.component.ts |
Applies and toggles landing views. |
list-item.component.spec.ts |
Tests default-view behavior. |
list-item.component.scss |
Styles form-default entries. |
list-item.component.html |
Adds the default-view action. |
workflow-persist.service.ts |
Sends default view during creation. |
workflow-persist.service.spec.ts |
Tests creation payloads. |
formly-config.ts |
Registers the editable-label wrapper. |
editable-label-wrapper.component.ts |
Implements editable field labels. |
editable-label-wrapper.component.spec.ts |
Tests label editing and hiding. |
editable-label-wrapper.component.scss |
Styles editable labels. |
editable-label-wrapper.component.html |
Renders label controls. |
GuiConfigSpec.scala |
Updates the enabled-default assertion. |
gui.conf |
Enables Form View by default. |
Review details
- Files reviewed: 34/34 changed files
- Comments generated: 11
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dcccaf1 to
3074a06
Compare
2006fad to
125e1a6
Compare
### What changes were proposed in this PR? Closes apache#8496. A regression from apache#8125: since then, every workflow's second autosave (and every one after it) fails with 500, silently on the operator canvas and as "Could not save" on the Form View. `persistWorkflow` wrote `is_public` from the request. The frontend feeds the saved row straight back as its metadata, and that row names the flag `isPublic` while the rest of the frontend calls it `isPublished`, so the very next save went out without the flag, the update wrote NULL into a NOT NULL column, and the request failed. A stale `isPublic: false` on a save could likewise un-publish a published workflow. - Backend: `saveWorkflowFields` now writes name, description and content only. Publishing stays with `/public` and `/private`, `default_view` with `/set-default-view`, and the timestamps are not rewritten, so a save can never clobber a concurrent change to any of them. - Frontend: `WorkflowPersistService.persistWorkflow` no longer sends `isPublic` (the endpoint does not read it, and the value is not reliably known after the first save), and `WorkflowUtilService.parseWorkflowInfo` carries a persist response's `isPublic` over to `isPublished`, so metadata fed back from a save keeps the publish state instead of dropping it. The Form View stack is not blocked by this: apache#8455 touches none of these files, and apache#8456 touches `workflow-persist.service.ts` only in `createWorkflow` (adding `defaultView`), a different function; a dry-run merge of the two is clean. ### Any related issues, documentation, discussions? Closes apache#8496. Found while verifying apache#8455 on a flag-on instance (parent apache#8011). ### How was this PR tested? Backend: `WorkflowResourceSpec` gains two tests, a save carrying no flag neither fails nor changes `is_public` after `/public`, and a save carrying `false` does not un-publish; the existing default-view save test was updated to send no flag, as the frontend does. `WorkflowResourceSpec` and `PublishedCopySchemaSpec` pass (87 tests), scalafmt clean. Frontend: the persist spec asserts the save payload carries no `isPublic` and that the response's `isPublic` comes back as `isPublished`; `parseWorkflowInfo` gains tests for the carry-over and for leaving a present `isPublished` alone. Full suite passes (5777), changed lines fully covered, eslint, prettier and the production (AOT) build pass. End to end, against a running stack rebuilt with this change: the exact second-save payload that returned 500 now returns 200 with `is_public` unchanged; a create-through-persist with the new payload inserts with `is_public = false`; and in a headless browser the Form View renames a workflow twice with every `/api/workflow/persist` answering 200, no `isPublic` key in any request body, and no "Could not save". ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Meng Wang <mengw15@uci.edu>
The workspace menu's export test module-mocked the CommonJS file-saver
package (vi.mock("file-saver")). Under the Angular unit-test builder that
call ends up inside the built spec chunk rather than at its top level
(Vitest warns about it on every platform), and once this stack shifted the
shared chunk graph the mock stopped applying on the Windows leg: the real
saveAs ran and the spy saw 0 calls (7 of the last 8 Windows runs on apache#8455
and apache#8456, while main and apache#8499 pass).
Route the download through the existing injectable FileSaverService, as
the dashboard downloads already do, and stub that with TestBed, which does
not depend on mock hoisting. No behaviour change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
125e1a6 to
379293f
Compare
The workspace menu's export test module-mocked the CommonJS file-saver
package (vi.mock("file-saver")). Under the Angular unit-test builder that
call ends up inside the built spec chunk rather than at its top level
(Vitest warns about it on every platform), and once this stack shifted the
shared chunk graph the mock stopped applying on the Windows leg: the real
saveAs ran and the spy saw 0 calls (7 of the last 8 Windows runs on apache#8455
and apache#8456, while main and apache#8499 pass).
Route the download through the existing injectable FileSaverService, as
the dashboard downloads already do, and stub that with TestBed, which does
not depend on mock hoisting. No behaviour change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
ff7d8e6 to
cef8521
Compare
cef8521 to
d8723f8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Read-only switching, save-drain races, stale computing-unit callbacks, and canvas export round-tripping remain incorrect.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
frontend/src/app/workspace/component/menu/menu.component.ts:671
- This dirty marker only observes graph/form-binding events; workflow-name edits emit
workflowMetaDataChanged()instead. If the name is committed while this save is in flight, its queued save starts after this one, but this completion immediately unloads the page and can abort it; the response also temporarily overwrites the new name. Preserve and mark a name that changed after the snapshot so the hand-over performs its final save before navigating.
// The snapshot below carries everything reported up to now.
this.editedSinceSwitchSnapshot = false;
this.workflowPersistService
.persistWorkflow(this.workflowActionService.getWorkflow())
- Files reviewed: 24/24 changed files
- Comments generated: 3
- Review effort level: Balanced
5db7eef to
366080c
Compare
|
/request-review: @mengw15 |
There was a problem hiding this comment.
🟡 Changes recommended
Stale save responses can lose edits, keyboard access is incomplete, and the promised feature rollout remains disabled.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 4
- Review effort level: Balanced
366080c to
fd84abc
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unsaved workflow IDs and metadata edits during hand-over can cause invalid navigation, spurious requests, or lost changes.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 3
- Review effort level: Balanced
fd84abc to
0baea8b
Compare
|
Two bookkeeping points before this merges:
|
Now that the Form View is complete, wire it into the app. The flag stays off; the stack's closing PR turns it on. A workflow opens in its default view: the dashboard card deep-links a form-default one straight into the form (with the Form View icon) and carries a per-card toggle to change the default via set-default-view; a canvas-default one is unchanged. Either view switches to the other at any time from the same control in the operator canvas menu. The page redirects to the canvas only when the flag is off. On the card the toggle sits in the action footer, in the same slot as on the row (after Detail); the row's hover-revealed action group also shows while the row has the keyboard focus, so the toggle can be reached without a pointer there too. The picked computing unit is remembered per workflow (localStorage) so it survives the full-page reload between the two views. Download-then-upload round-trips the landing view: the exported JSON carries default_view as a sibling of the content (one shared export shape, used by the dashboard download and the canvas menu's export alike), and upload pulls it back onto the workflow row rather than into the content. Saves now go out one at a time and in call order (WorkflowPersistService), so the switch's save lands after an autosave already in flight; an edit made while the switch's save is out is saved once more before the hand-over; a reader goes straight over without a save; a second click mid hand-over is a no-op; a workflow never saved yet (the default id) is created by the switch's save and the hand-over opens the id it was given. Only the user's own pick of a computing unit is remembered, not the units selected on load, and a lookup that answers after the workflow changed underneath it is dropped. The dashboard toggle handlers check WRITE access themselves. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
mengw15
left a comment
There was a problem hiding this comment.
LGTM, thanks for addressing the comments
0baea8b to
8c837a4
Compare
What changes were proposed in this PR?
Closes #8028. Part of the Form View stack (parent issue #8011), on main now that #8516 and #8517 have merged. The review commit is the branch's single commit.
Wires the Form View entry points. The flag stays off here; the stack's closing PR, #8528, flips it.
default_view(form or canvas), with a toggle that persists the choice; the deep link goes to the existing/workflow/:id/formroute. Both renderers of the dashboard, the list row and the card, follow one shared rule (default-view-landing.ts: mark, deep link, toggle), so switching the view mode does not lose the entry point. The toggle is offered only with WRITE access, which the endpoint requires, and the handler checks the same rule rather than trusting the template; it is a proper toggle button (constant accessible name, state inaria-pressed, the hover title spelling out what a click does); hub links are left untouched.WorkflowPersistServicenow sends saves one at a time and in call order, at the one place every save goes through, so the switch's save lands and completes after it; each caller still gets only its own result and a failed save does not hold up the next), and an edit made while the switch's save is out (the page stays editable until the load;workflowChangedmarks it and the hand-over saves once more before leaving). A reader, who cannot save, goes straight over. A workflow the canvas holds but has never saved (the default id) is created by that save, and the hand-over opens the id the save answered with. On the card the toggle sits in the always-visible action footer, in the same slot as on the row (right after Detail); the row's hover-revealed action group also appears while the row has the keyboard focus, so the toggle can be reached without a pointer there too. A second click while the hand-over is in progress is a no-op. A failed save keeps the user on the canvas with the error shown. Every workflow offers both views whenever the flag is on:default_viewonly decides the landing view, and neither view gates the other.defaultViewas a sibling key next to the workflow content, in one shared export shape (exportedWorkflow) used by the dashboard download and the canvas menu's export alike; an old export without the key imports unchanged.Any related issues, documentation, discussions?
Closes #8028. Part of the Form View feature (parent issue #8011).
How was this PR tested?
Unit tests (vitest) cover the menu's Canvas / Form View switch through the DOM (absent with the flag off, Canvas pressed, Form View handing over, hidden while an older version is displayed), the row's and the card's default-view behavior (mark and deep link, hub link untouched, flag off leaves the dashboard as today, WRITE-only toggle in the DOM, toggle on / off / failed request / no cached row), the menu switch (navigates only once the save completes, stays on the canvas with the error when it fails, saves once more when an edit lands while its save is out, takes a reader straight over without a save, ignores a second click mid hand-over), the canvas export carrying
defaultViewnext to the content and omitting it when unset, the hand-over opening the id the save assigned when the canvas held a never-saved workflow, the persist service sending saves one at a time in order with each caller getting its own result and a failure not holding up the next, the dashboard toggle handlers refusing without WRITE access, the toggle's aria-pressed following the state, the export/import round-trip including a legacy file withoutdefaultView, and the computing-unit recall (waits for the first non-empty unit list, forgets a terminated unit and falls back, drops a stale decision after the workflow changed, a late last-execution answer or fallback included, positive-integer validation, storage failures, only an explicit pick remembered). Each new guard was deletion-checked (removing it turns the corresponding test red). eslint, prettier and the production (AOT) build pass; every changed line, template lines included, is statement and function covered.Video
1. Default view on the dashboard
(row and card toggle, Form View icon, deep link into the form, toggle off again, no toggle without write access)
Screen.Recording.2026-09-12.at.9.00.54.PM.mov
2. Canvas / Form View switch
(save first, then the hand-over; and back)
Screen.Recording.2026-09-12.at.9.01.40.PM.mov
3. Computing unit remembered across the switch
Screen.Recording.2026-09-12.at.9.02.37.PM.mov
4. Download / upload keeps the default view
Screen.Recording.2026-09-12.at.11.34.15.PM.mov
5. Default view on the card view
(the same toggle in the card's action row, Form View icon, deep link into the form)
Screen.Recording.2026-09-13.at.4.45.51.PM.mov
Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY