fix: inherit the workflow's application version and build on toggle relaxation - #166
Open
VsevolodX wants to merge 1 commit into
Open
fix: inherit the workflow's application version and build on toggle relaxation#166VsevolodX wants to merge 1 commit into
VsevolodX wants to merge 1 commit into
Conversation
VsevolodX
force-pushed
the
feature/SOF-7884
branch
from
August 28, 2026 19:03
e6d452a to
3699c5e
Compare
…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
VsevolodX
force-pushed
the
feature/SOF-7884
branch
from
August 28, 2026 19:05
3699c5e to
0ddf524
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
"Toggle relaxation" inserted a relaxation subworkflow carrying the standata template's own hardcoded
application (espresso
6.3/GNU, vasp5.4.4/GNU) instead of the one the workflow already runs.toggleRelaxation()read the existing application only to look the template up by name, thendiscarded it.
Version and Build then rendered blank:
avebuilds those option lists fromApplicationRegistry.getApplications()filtered by name (and, for Build, by version too), and avalue 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 thesubworkflow 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 readbefore insertion because
addSubworkflow(_, true)unshifts, sosubworkflowInstances[0]is adifferent subworkflow afterwards.
getStandataRelaxationSubworkflow()no longer returnsapplication: 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.ts—buildwas"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:
unitsInstanceswithout re-serializingunits—setUnitsis the only place thatserializes them and
setApplicationnever calls it. Since workflow state is stored as JSON andre-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-hydrated6.3/GNU.method.subtypefrom paw to us — the wrong pseudopotential scheme for vasp, and a silentchange to what the job computes.
setDefaultInput(), so staleinput[].templateentries 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.tsone-liner is part of the fix, not a drive-bybuild: "6.3"was inert before this change — executable resolution keys on name and version andignores
buildentirely. Once the value is inherited, it is stamped onto the relaxation subworkflowand its unit, and
ave's Build options for espresso 6.3 are["GNU"]— so the control renderedblank. That is this ticket's own symptom, reintroduced on the
/bank/workflows/newpath, whichbuilds from
defaultConfig. Caught in review, after a green test suite and a green browser run hadboth missed it.
Guarded now by
inherits a version and build the registry actually offers, which fails if the badvalue comes back.
Noted, not fixed
Subworkflow.setApplicationdoes not re-serializeunits— affects the manual Version dropdown.getStandataRelaxationSubworkflowdereferencessubworkflowInstances[0]unguarded, sotoggleRelaxation()on a workflow with no subworkflows already throws.tests/js/Workflow.test.tsalone reports one failure that the full suite does not:ApplicationRegistry.setDriveris static anddescribe("addUnitType")never sets it.SOF-7884
🤖 Generated with Claude Code