Add workflow-level variables for DRYing up repeated strings in specs#308
Add workflow-level variables for DRYing up repeated strings in specs#308daniel-thom merged 5 commits intomainfrom
variables for DRYing up repeated strings in specs#308Conversation
Regenerated `.sqlx/` from the current code via `cargo sqlx prepare --workspace -- --all-features --all-targets`. The cache had drifted from the source -- 12 stale query files were removed and 8 newly introduced queries were added -- which was breaking `SQLX_OFFLINE=true cargo clippy --all-features` and the pre-commit hook on environments without a populated dev database. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Variables are workflow-level constants substituted via {name} into every
string field of the spec before parameter expansion. They differ from
`parameters` in that each reference is replaced once with the variable's
value -- they do not trigger Cartesian expansion. Variables and
parameters compose freely in the same string.
Validation rejects variable/parameter name collisions and undefined
{name} references (typos). KDL parser and serializer learn the new
top-level node. Examples added in YAML, JSON, JSON5, and KDL; docs
updated in parameterization.md and workflow-spec.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a workflow-level variables map to the workflow spec so repeated constant strings can be substituted via {name} across the entire spec prior to parameter expansion, with format support and typo/collision validation. This also updates the KDL parser/serializer, adds docs + examples for all formats, and refreshes the .sqlx/ offline query cache.
Changes:
- Implement workflow-level
variablessubstitution + validation during spec loading (before deserialization / parameter expansion). - Add
variables_demoexamples for YAML/JSON/JSON5/KDL and expand unit test coverage around variable substitution. - Update docs to describe workflow variables, syntax across formats, and best practices; refresh
.sqlxoffline cache artifacts.
Reviewed changes
Copilot reviewed 11 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/client/workflow_spec.rs | Implements variables substitution/validation, extends spec model + KDL parsing/serialization, and adds tests. |
| examples/yaml/variables_demo.yaml | Adds a YAML example demonstrating workflow-level variables. |
| examples/kdl/variables_demo.kdl | Adds a KDL example demonstrating workflow-level variables. |
| examples/json/variables_demo.json | Adds a JSON example demonstrating workflow-level variables. |
| examples/json/variables_demo.json5 | Adds a JSON5 example demonstrating workflow-level variables. |
| docs/src/core/reference/workflow-spec.md | Documents the new top-level variables field. |
| docs/src/core/reference/parameterization.md | Adds a new “Workflow Variables” section and best-practice guidance. |
| .sqlx/query-f0c87c852c9acb25674a1cdde06ec105d8822b4cbc27823bc212e7fc11c8dc79.json | Updates offline SQLx cache (query metadata). |
| .sqlx/query-eff9aa80713f24c3e8514c09e6648695dbe5f474bacd1303c7f423abe091b774.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-e13329ec5ca6682229d5c3ce42f02a8c62d633ce710194ecb7dab005f219bcf0.json | Updates offline SQLx cache (query changed). |
| .sqlx/query-d9899cd7b0730f91426077f3f5cafbf596e9ba045ab3ad1f66a752d631600254.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-d89de81f33fa3ad43f7f7c4f0393c3db2aabb55b6f57fde91eb8b109e6a6ac5c.json | Adds offline SQLx cache entry for a new/updated query. |
| .sqlx/query-c3376e741935563a695871ef16db6ac3f7529b108df47290496a0c255507868d.json | Updates offline SQLx cache (query changed). |
| .sqlx/query-b7a95f913fbf7b6f499da3a808a30ae29e0959e82efea4ed4e4b805a874005f0.json | Adds offline SQLx cache entry for a new/updated query. |
| .sqlx/query-a71074c980b1b05131a55552965d30a78b2b95b9cd53a470f033503a6318da34.json | Adds offline SQLx cache entry for a new/updated query. |
| .sqlx/query-9bc5fffe7a49355df5e7d98fd23e0e452ac29673fa6f7d5025b96bf492394b1a.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-985c3a022488eab321c29d7ea1e1480b17c768a464fa67f8c68675a49ef18c77.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-6e47fe4e806c69a1630ad2b7fc0c49fd0f66d1471d906ee931bcd478b520600d.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-67e128d9ce384ee620e239c70ef555a6b8d0ea2f39443f0bdf739778a1af49dd.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-3a988070b4c92d180d18de847a61bdee811afa2abdb5224d8b5b279c6dbd5593.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-2a0bf40600d0a6a28a8bc33f482d495efad48a853df95ab87a4b4ef421ce19e1.json | Updates offline SQLx cache (query changed). |
| .sqlx/query-212dc8ede4808ff5a0f1f827e46247ac6a8796193c04e974b04280522bba9702.json | Adds offline SQLx cache entry for a new/updated query. |
| .sqlx/query-17b609df5736bdd3a5f512c882c0fdf1eebd528d6c32cc4ecd13ff506a5fdfbf.json | Removes stale offline SQLx cache entry. |
| .sqlx/query-17b0ff689068143898c9c217439cbffe59e443af89bb375e9f7f306dc0f0321b.json | Updates offline SQLx cache (query changed). |
Files not reviewed (13)
- .sqlx/query-17b609df5736bdd3a5f512c882c0fdf1eebd528d6c32cc4ecd13ff506a5fdfbf.json: Language not supported
- .sqlx/query-212dc8ede4808ff5a0f1f827e46247ac6a8796193c04e974b04280522bba9702.json: Language not supported
- .sqlx/query-3a988070b4c92d180d18de847a61bdee811afa2abdb5224d8b5b279c6dbd5593.json: Language not supported
- .sqlx/query-67e128d9ce384ee620e239c70ef555a6b8d0ea2f39443f0bdf739778a1af49dd.json: Language not supported
- .sqlx/query-6e47fe4e806c69a1630ad2b7fc0c49fd0f66d1471d906ee931bcd478b520600d.json: Language not supported
- .sqlx/query-985c3a022488eab321c29d7ea1e1480b17c768a464fa67f8c68675a49ef18c77.json: Language not supported
- .sqlx/query-9bc5fffe7a49355df5e7d98fd23e0e452ac29673fa6f7d5025b96bf492394b1a.json: Language not supported
- .sqlx/query-a71074c980b1b05131a55552965d30a78b2b95b9cd53a470f033503a6318da34.json: Language not supported
- .sqlx/query-b7a95f913fbf7b6f499da3a808a30ae29e0959e82efea4ed4e4b805a874005f0.json: Language not supported
- .sqlx/query-d89de81f33fa3ad43f7f7c4f0393c3db2aabb55b6f57fde91eb8b109e6a6ac5c.json: Language not supported
- .sqlx/query-d9899cd7b0730f91426077f3f5cafbf596e9ba045ab3ad1f66a752d631600254.json: Language not supported
- .sqlx/query-eff9aa80713f24c3e8514c09e6648695dbe5f474bacd1303c7f423abe091b774.json: Language not supported
- .sqlx/query-f0c87c852c9acb25674a1cdde06ec105d8822b4cbc27823bc212e7fc11c8dc79.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Skip `${...}` blocks in undefined-token validation. Shell-style variable
expansion (and the repo's existing `${files.input.X}` / `${TORC_*}`
references) would otherwise trigger a false-positive
"undefined template name" error whenever a workflow defined `variables`.
- Run undefined-token validation on variable values themselves. A typo
like `{baes_path}` inside a variable's value used to slip through
silently and only surface as a confusing error wherever the variable
was referenced.
Adds regression tests for both: `${TORC_JOB_ID}` / `${HOME}` in commands
and env values must coexist with `variables`, and a typo inside a
variable value must be rejected at load time.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 24 changed files in this pull request and generated 2 comments.
Files not reviewed (13)
- .sqlx/query-17b609df5736bdd3a5f512c882c0fdf1eebd528d6c32cc4ecd13ff506a5fdfbf.json: Language not supported
- .sqlx/query-212dc8ede4808ff5a0f1f827e46247ac6a8796193c04e974b04280522bba9702.json: Language not supported
- .sqlx/query-3a988070b4c92d180d18de847a61bdee811afa2abdb5224d8b5b279c6dbd5593.json: Language not supported
- .sqlx/query-67e128d9ce384ee620e239c70ef555a6b8d0ea2f39443f0bdf739778a1af49dd.json: Language not supported
- .sqlx/query-6e47fe4e806c69a1630ad2b7fc0c49fd0f66d1471d906ee931bcd478b520600d.json: Language not supported
- .sqlx/query-985c3a022488eab321c29d7ea1e1480b17c768a464fa67f8c68675a49ef18c77.json: Language not supported
- .sqlx/query-9bc5fffe7a49355df5e7d98fd23e0e452ac29673fa6f7d5025b96bf492394b1a.json: Language not supported
- .sqlx/query-a71074c980b1b05131a55552965d30a78b2b95b9cd53a470f033503a6318da34.json: Language not supported
- .sqlx/query-b7a95f913fbf7b6f499da3a808a30ae29e0959e82efea4ed4e4b805a874005f0.json: Language not supported
- .sqlx/query-d89de81f33fa3ad43f7f7c4f0393c3db2aabb55b6f57fde91eb8b109e6a6ac5c.json: Language not supported
- .sqlx/query-d9899cd7b0730f91426077f3f5cafbf596e9ba045ab3ad1f66a752d631600254.json: Language not supported
- .sqlx/query-eff9aa80713f24c3e8514c09e6648695dbe5f474bacd1303c7f423abe091b774.json: Language not supported
- .sqlx/query-f0c87c852c9acb25674a1cdde06ec105d8822b4cbc27823bc212e7fc11c8dc79.json: Language not supported
- Forbid variable values from referencing other variables. The previous
implementation called `substitute_parameters` over the variables
HashMap, which iterates in randomized order; nested variable
references like `inputs: "{base}/sub"` would resolve or leak the
inner token depending on iteration order. Variable values may still
reference parameter names (e.g. `i: "1:{n_max}"`), since that
substitution path is unaffected.
- Validate variable names are valid identifiers
(`[A-Za-z_][A-Za-z0-9_]*`). Non-identifier names previously skipped
typo detection silently and would have produced invalid KDL output.
Both rules error at spec-load time with messages naming the offending
variable. Two new tests cover the rejected cases; docs updated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 24 changed files in this pull request and generated 2 comments.
Files not reviewed (13)
- .sqlx/query-17b609df5736bdd3a5f512c882c0fdf1eebd528d6c32cc4ecd13ff506a5fdfbf.json: Language not supported
- .sqlx/query-212dc8ede4808ff5a0f1f827e46247ac6a8796193c04e974b04280522bba9702.json: Language not supported
- .sqlx/query-3a988070b4c92d180d18de847a61bdee811afa2abdb5224d8b5b279c6dbd5593.json: Language not supported
- .sqlx/query-67e128d9ce384ee620e239c70ef555a6b8d0ea2f39443f0bdf739778a1af49dd.json: Language not supported
- .sqlx/query-6e47fe4e806c69a1630ad2b7fc0c49fd0f66d1471d906ee931bcd478b520600d.json: Language not supported
- .sqlx/query-985c3a022488eab321c29d7ea1e1480b17c768a464fa67f8c68675a49ef18c77.json: Language not supported
- .sqlx/query-9bc5fffe7a49355df5e7d98fd23e0e452ac29673fa6f7d5025b96bf492394b1a.json: Language not supported
- .sqlx/query-a71074c980b1b05131a55552965d30a78b2b95b9cd53a470f033503a6318da34.json: Language not supported
- .sqlx/query-b7a95f913fbf7b6f499da3a808a30ae29e0959e82efea4ed4e4b805a874005f0.json: Language not supported
- .sqlx/query-d89de81f33fa3ad43f7f7c4f0393c3db2aabb55b6f57fde91eb8b109e6a6ac5c.json: Language not supported
- .sqlx/query-d9899cd7b0730f91426077f3f5cafbf596e9ba045ab3ad1f66a752d631600254.json: Language not supported
- .sqlx/query-eff9aa80713f24c3e8514c09e6648695dbe5f474bacd1303c7f423abe091b774.json: Language not supported
- .sqlx/query-f0c87c852c9acb25674a1cdde06ec105d8822b4cbc27823bc212e7fc11c8dc79.json: Language not supported
…lue rule
- Substitution of workflow variables previously used `substitute_parameters`,
which does naive `string.replace` and would corrupt `\${HOME}` into
`\$<value>` if a variable shared the name `HOME`. The new
`substitute_workflow_variables_in_string` is a single-pass scanner that
skips `\${...}` blocks, so shell-style expansion is preserved verbatim
even when names collide.
- Tighten the variable-value rule: variable values must be plain literal
strings. Any `{name}` template reference inside a variable's value is
now rejected (whether it points at another variable, a parameter, or a
typo). This keeps semantics simple and removes a misleading
inline example. Shell-style `\${...}` is still allowed and preserved.
- Fix the misleading example in the inline comment and in
`parameterization.md`: `i: "1:{n_max}"` is a parameter value
referencing a variable, not a variable value referencing a parameter.
Two new tests cover the cases:
- a variable named `HOME` must not corrupt `\${HOME}` in commands or env
- a parameter reference inside a variable value must be rejected
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 24 changed files in this pull request and generated 3 comments.
Files not reviewed (13)
- .sqlx/query-17b609df5736bdd3a5f512c882c0fdf1eebd528d6c32cc4ecd13ff506a5fdfbf.json: Language not supported
- .sqlx/query-212dc8ede4808ff5a0f1f827e46247ac6a8796193c04e974b04280522bba9702.json: Language not supported
- .sqlx/query-3a988070b4c92d180d18de847a61bdee811afa2abdb5224d8b5b279c6dbd5593.json: Language not supported
- .sqlx/query-67e128d9ce384ee620e239c70ef555a6b8d0ea2f39443f0bdf739778a1af49dd.json: Language not supported
- .sqlx/query-6e47fe4e806c69a1630ad2b7fc0c49fd0f66d1471d906ee931bcd478b520600d.json: Language not supported
- .sqlx/query-985c3a022488eab321c29d7ea1e1480b17c768a464fa67f8c68675a49ef18c77.json: Language not supported
- .sqlx/query-9bc5fffe7a49355df5e7d98fd23e0e452ac29673fa6f7d5025b96bf492394b1a.json: Language not supported
- .sqlx/query-a71074c980b1b05131a55552965d30a78b2b95b9cd53a470f033503a6318da34.json: Language not supported
- .sqlx/query-b7a95f913fbf7b6f499da3a808a30ae29e0959e82efea4ed4e4b805a874005f0.json: Language not supported
- .sqlx/query-d89de81f33fa3ad43f7f7c4f0393c3db2aabb55b6f57fde91eb8b109e6a6ac5c.json: Language not supported
- .sqlx/query-d9899cd7b0730f91426077f3f5cafbf596e9ba045ab3ad1f66a752d631600254.json: Language not supported
- .sqlx/query-eff9aa80713f24c3e8514c09e6648695dbe5f474bacd1303c7f423abe091b774.json: Language not supported
- .sqlx/query-f0c87c852c9acb25674a1cdde06ec105d8822b4cbc27823bc212e7fc11c8dc79.json: Language not supported
| return Err(format!( | ||
| "undefined template name '{{{}}}' in '{}': not declared in `variables` \ | ||
| or any `parameters` map. Add it to `variables` or fix the typo.", | ||
| inner, s | ||
| ) |
| "variable '{}' value '{}' references another variable '{{{}}}'. \ | ||
| Variable values may not reference other variables; resolution \ | ||
| order would be undefined. Inline the constant or compose at the \ | ||
| use site.", | ||
| var_name, s, inner | ||
| ) | ||
| .into()) | ||
| } else { | ||
| Err(format!( | ||
| "variable '{}' value '{}' contains template reference '{{{}}}'. \ | ||
| Variable values must be plain literal strings (shell-style \ | ||
| `${{...}}` is allowed). Compose at the use site instead.", | ||
| var_name, s, inner |
| Err(format!( | ||
| "variable '{}' value '{}' references another variable '{{{}}}'. \ | ||
| Variable values may not reference other variables; resolution \ | ||
| order would be undefined. Inline the constant or compose at the \ | ||
| use site.", | ||
| var_name, s, inner | ||
| ) | ||
| .into()) |
Summary
variablesmap: constants substituted via{name}into every string field of the spec before parameter expansion. Unlikeparameters, variables do not trigger Cartesian expansion -- each reference is replaced once. Variables and parameters compose freely in the same string.{name}references (typo detection). Tokens with non-identifier inner text (e.g.find ... {} \;, JSON-like fragments) are intentionally ignored.variables { ... }node so the feature works uniformly across YAML, JSON, JSON5, and KDL..sqlx/offline cache as a separate prep commit (12 stale queries removed, 8 newly-introduced queries added). The cache had drifted from source onmain, which was breaking offline-mode clippy and the pre-commit hook for anyone without a populated dev database.parameterization.md(with KDL/JSON5 syntax and a "when to reach for variables vs. shared parameters" guide), new field row inworkflow-spec.md, and a best-practice tip.variables_demoworkflow inexamples/yaml/,examples/json/(both.jsonand.json5), andexamples/kdl/-- each demonstrating variables in env values, file paths, job commands composing with sweep parameters, and Slurm scheduleraccountfields.Test plan
{base_path}/job_{i:03d}.out)i: "1:{n_max}")variablesmapfrom_spec_fileloads each format'svariables_demoexample and produces the expected substituted strings (4 tests, one per format)cargo fmt --check,cargo clippy --all --all-targets --all-features -- -D warnings, anddprint checkall pass via the pre-commit hook🤖 Generated with Claude Code