feat(wanda): Add 'params' field for env var validation and discovery

[andrew-anyscale]

Add params field to wanda spec for declaring allowed values of environment variables used in templated fields (name, froms). This enables:

• Strict validation: reject builds where env var values don't match
  declared params
• Dependency discovery: specs with params are indexed under all their
  expanded names (e.g., base$PY with params [3.10, 3.11] is indexed as
  both base3.10 and base3.11), enabling dependency resolution without
  requiring all env vars to be set

Validation runs before variable expansion in buildDepGraph, so errors
reference the original $VARNAME.

  name: rayml$PY_VERSION
  params:
    PY_VERSION:
      - "3.10"
      - "3.11"
      - "3.12"
  froms:
    - cr.ray.io/rayproject/base$PY_VERSION
  dockerfile: rayml.Dockerfile

  # Valid - succeeds
  PY_VERSION=3.11 wanda rayml.wanda.yaml

  # Invalid - fails with validation error
  PY_VERSION=3.9 wanda rayml.wanda.yaml
  # Error: env var PY_VERSION has value "3.9", but params only allow: [3.10 3.11 3.12]

Topic: wanda-params
Relative: wanda-deps

Signed-off-by: andrew andrew@anyscale.com

[andrew-anyscale]

Reviews in this chain:
└#368 feat(wanda): Add 'params' field for env var validation and discovery

[andrew-anyscale]

#	head	base	diff	date	summary
0	f8311352	388c9c17	diff	Jan 12 17:04 PM	4 files changed, 553 insertions(+), 5 deletions(-)
1	0337224c	05c0ded9	diff	Jan 12 17:12 PM	2 files changed, 58 insertions(+), 33 deletions(-)
2	cd893e4c	839b0867	diff	Jan 12 17:22 PM	1 file changed, 1 insertion(+), 1 deletion(-)
3	aa778408	839b0867	diff	Jan 12 17:29 PM	2 files changed, 33 insertions(+), 42 deletions(-)
4	c1be0808	308598c1	rebase	Jan 12 17:41 PM	0 files changed
5	982cc460	e8c1f1b2	rebase	Jan 12 17:49 PM	0 files changed
6	e1c5fedc	a8a69082	rebase	Jan 12 18:07 PM	0 files changed
7	aa3f891a	4d7b3449	rebase	Jan 12 18:15 PM	0 files changed
8	590a1f98	b8380304	rebase	Jan 12 18:24 PM	0 files changed
9	a0cda8c9	a289e514	rebase	Jan 12 18:25 PM	0 files changed
10	91301758	2327511e	rebase	Jan 12 18:37 PM	0 files changed
11	7367abfc	fdbd39ef	rebase	Jan 12 18:49 PM	0 files changed
12	0c9f9450	86b1c2e1	rebase	Jan 12 18:57 PM	0 files changed
13	6539ed83	00b3ca37	rebase	Jan 12 19:22 PM	0 files changed
14	bbd00709	00b3ca37	diff	Jan 12 19:30 PM	1 file changed, 3 insertions(+), 3 deletions(-)
15	3a439d33	1a7af24d	diff	Jan 13 8:30 AM	2 files changed, 24 insertions(+), 66 deletions(-)
16	44d91e73	b0597a44	rebase	Jan 13 8:45 AM	0 files changed
17	59be6945	c0d07aea	rebase	Jan 13 8:49 AM	0 files changed
18	922cdab9	e20bf2d1	rebase	Jan 13 9:32 AM	0 files changed
19	32e8cb9d	7c9b96c9	diff	Jan 13 11:43 AM	1 file changed, 6 insertions(+), 4 deletions(-)
20	5ca62ee2	cd5a547a	diff	Jan 13 11:51 AM	1 file changed, 4 insertions(+), 4 deletions(-)
21	56081ddb	987fd295	rebase	Jan 13 11:56 AM	0 files changed
22	c63731b9	543dfc49	diff	Jan 13 16:28 PM	1 file changed, 8 insertions(+), 9 deletions(-)
23	80ff2eae	5a582a1d	rebase	Jan 13 16:30 PM	0 files changed
24	fafb9d9f	35530d6d	rebase	Jan 13 16:48 PM	0 files changed
25	b4efcec4	a384fdf6	rebase	Jan 13 17:23 PM	0 files changed
26	54087176	df64f501	diff	Jan 13 21:39 PM	0 files changed

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[gitar-bot]

Code Review 👍 Approved with suggestions 0 resolved / 3 findings

Well-designed feature with comprehensive test coverage. A few minor robustness improvements suggested around edge cases.

Suggestions 💡 3 suggestions

Edge Case: extractVarNames parsing differs from test expectations

📄 wanda/spec.go:209

Looking at the test case "prefix$VAR1-$VAR2suffix" which expects ["VAR1", "VAR2suffix"], this suggests that variables without explicit termination (like - after VAR1) correctly stop at non-alphanumeric characters, but VAR2suffix shows that suffix is being included as part of the variable name.

This is correct behavior per the parsing logic (letters/underscores continue the variable name), but the test expectation reveals a potential usability concern: users might expect $VAR2suffix to expand $VAR2 only and leave suffix as literal text. This is an edge case worth documenting, though the behavior is internally consistent with how the parser works.

Edge Case: Potential exponential blowup in cartesian product generation

📄 wanda/spec.go:210

The expandVarWithParams function generates the cartesian product of all param combinations. With multiple variables each having many values, this could produce a very large number of combinations.

For example, with 3 variables each having 10 values, this would generate 1000 combinations. While this is likely acceptable for typical use cases (2-3 Python versions, 2 architectures), there's no upper bound or warning for unusual configurations.

Consider adding a safeguard or warning if the number of combinations exceeds a reasonable threshold (e.g., 100), to help users catch misconfiguration early:

const maxCombinations = 100
if len(combinations) > maxCombinations {
    return nil, fmt.Errorf("params generate %d combinations, exceeding limit of %d", len(combinations), maxCombinations)
}

Code Quality: Map iteration order in ValidateParams is non-deterministic

📄 wanda/spec.go:211 📄 wanda/spec.go:156

In ValidateParams, the function iterates over s.Params map. Since Go map iteration order is non-deterministic, if multiple params have invalid values, the error message will vary between runs, potentially reporting different variables each time.

While this doesn't affect correctness (any validation failure is still a failure), it could make debugging harder since users might see different error messages for the same configuration.

Consider collecting all validation errors or sorting the keys before iteration:

func (s *Spec) ValidateParams(lookup lookupFunc) error {
    var errs []string
    for varName, allowed := range s.Params {
        val, ok := lookup(varName)
        if !ok {
            continue
        }
        valid := false
        for _, a := range allowed {
            if val == a {
                valid = true
                break
            }
        }
        if !valid {
            errs = append(errs, fmt.Sprintf("%s=%q (allowed: %v)", varName, val, allowed))
        }
    }
    if len(errs) > 0 {
        sort.Strings(errs)
        return fmt.Errorf("invalid param values: %s", strings.Join(errs, ", "))
    }
    return nil
}

What Works Well

Clean implementation of the params feature with clear separation between validation and expansion. Thorough test coverage including edge cases like cartesian products, deduplication, and env var fallback. Excellent error messages that include valid values to guide users.

Options

Auto-apply is off Gitar will not commit updates to this branch.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)}

[aslonnie]

maybe chat, but how does it handle free-formed string type env var expansion now?