Skip to content

fix: inherit the workflow's application version and build on toggle relaxation - #166

Open
VsevolodX wants to merge 1 commit into
mainfrom
feature/SOF-7884
Open

fix: inherit the workflow's application version and build on toggle relaxation#166
VsevolodX wants to merge 1 commit into
mainfrom
feature/SOF-7884

Conversation

@VsevolodX

@VsevolodX VsevolodX commented Aug 24, 2026

Copy link
Copy Markdown
Member

Summary

"Toggle relaxation" inserted a relaxation subworkflow carrying the standata template's own hardcoded
application (espresso 6.3/GNU, vasp 5.4.4/GNU) instead of the one the workflow already runs.
toggleRelaxation() read the existing application only to look the template up by name, then
discarded it.

Version and Build then rendered blank: ave builds those option lists from
ApplicationRegistry.getApplications() filtered by name (and, for Build, by version too), and a
value absent from the list shows as nothing selected.

Per the direction on the ticket — "since we are adding relaxation into an existing workflow, it
should just have the same version as the app that's already used inside this workflow"
— the
inserted subworkflow now inherits the workflow's application.

What changed

  • Subworkflow.createWithApplication(config, application) — stamps the application at the
    subworkflow level and on every execution unit, then constructs. Building from the config means the
    constructor re-resolves each unit's executable, flavor and input templates against the
    inherited version, and serializes the result.
  • Workflow.toggleRelaxation() passes the application the workflow already runs. It is read
    before insertion because addSubworkflow(_, true) unshifts, so subworkflowInstances[0] is a
    different subworkflow afterwards.
  • getStandataRelaxationSubworkflow() no longer returns application: executionUnit.application.
    That was the hardcoded value this PR exists to stop using, and the only caller overrode it
    immediately — so it was both dead and misleading.
  • workflows/default.tsbuild was "6.3", the version string in the build field. See below.

Deliberately not Subworkflow.setApplication()

That is the path the manual Version dropdown uses, and it was the obvious choice. It is wrong here on
three counts:

  1. It mutates unitsInstances without re-serializing unitssetUnits is the only place that
    serializes them and setApplication never calls it. Since workflow state is stored as JSON and
    re-hydrated, the inherited version survives in memory and is lost on the first round-trip.
    Measured: live instance 7.5/Intel, toJSON() 6.3/GNU, re-hydrated 6.3/GNU.
  2. It replaces the model with the application default, swapping the vasp relaxation template's
    method.subtype from paw to us — the wrong pseudopotential scheme for vasp, and a silent
    change to what the job computes.
  3. It never reaches setDefaultInput(), so stale input[].template entries survive a version change.

A first cut of this fix used setApplication, passed its unit test, and failed in the browser.
The setter's serialization gap still affects the manual dropdown path and wants its own ticket.

The default.ts one-liner is part of the fix, not a drive-by

build: "6.3" was inert before this change — executable resolution keys on name and version and
ignores build entirely. Once the value is inherited, it is stamped onto the relaxation subworkflow
and its unit, and ave's Build options for espresso 6.3 are ["GNU"] — so the control rendered
blank. That is this ticket's own symptom, reintroduced on the /bank/workflows/new path, which
builds from defaultConfig. Caught in review, after a green test suite and a green browser run had
both missed it.

Guarded now by inherits a version and build the registry actually offers, which fails if the bad
value comes back.

Noted, not fixed

  • Subworkflow.setApplication does not re-serialize units — affects the manual Version dropdown.
  • getStandataRelaxationSubworkflow dereferences subworkflowInstances[0] unguarded, so
    toggleRelaxation() on a workflow with no subworkflows already throws.
  • Running tests/js/Workflow.test.ts alone reports one failure that the full suite does not:
    ApplicationRegistry.setDriver is static and describe("addUnitType") never sets it.

SOF-7884

🤖 Generated with Claude Code

…elaxation [release]

toggleRelaxation() looked the relaxation subworkflow up from standata by application
name only, then stamped the template's own hardcoded application onto the inserted
subworkflow — espresso 6.3/GNU, vasp 5.4.4/GNU. The version and build already in use
by the workflow were read for the lookup and then discarded, so Version and Build
rendered blank whenever the deployment's registry no longer offered the template's
hardcoded pair: ave builds those option lists from ApplicationRegistry filtered by name
and version, and a value absent from the list shows as nothing selected.

Stamp the workflow's application into the subworkflow config, at the subworkflow level
and on every execution unit, and construct from that. The constructor then re-resolves
each unit's executable, flavor and input templates against the inherited version and
serializes the result. Read it before inserting: addSubworkflow(_, true) unshifts, so
subworkflowInstances[0] is a different subworkflow afterwards.

Deliberately not Subworkflow.setApplication(), despite that being the path the manual
Version dropdown uses. It updates unit instances without re-serializing `units`, so the
inherited version is lost through the JSON round-trip web-app's store performs; it
replaces the model with the application default, swapping the vasp relaxation template's
method.subtype from paw to us; and it never reaches setDefaultInput(), so stale input
templates survive a version change. The setter's serialization gap still affects the
manual dropdown and needs its own ticket.

getStandataRelaxationSubworkflow() no longer returns the execution unit's hardcoded
application. The caller stamps the inherited one, and the other caller matches on
systemName, so the field was both dead and misleading.

Also correct workflows/default.ts, whose application was a hand-written stub disagreeing
with the registry in three fields: build "6.3" (a version string where a build name
belongs), shortName "espresso" and summary "Quantum Espresso". Nothing resolved them
before — executable lookup keys on name and version — but inheriting build "6.3" stamped
a non-registry build onto the relaxation subworkflow, and ave's Build options for
espresso 6.3 are ["GNU"], so the control rendered blank. That is this ticket's own
symptom on /bank/workflows/new, which builds from defaultConfig. With the stub corrected
the pre-existing assertion in Workflow.test.ts passes unchanged.

Verified across every application the registry offers, asserting after a toJSON()
round-trip rather than on live instances: espresso (4 builds) and vasp inherit at the
subworkflow and unit level, both controls render a pair the registry offers,
executable/flavor resolve, and each keeps its template's method; the seven applications
with no relaxation template still toggle cleanly.

SOF-7884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant