fix: coerce LIST[BOOL] items per RFC 0007 §2.15 - #352
Merged
wyongzhi merged 4 commits intoSep 3, 2026
Conversation
Each LIST[BOOL] parameter item independently accepts the same values as scalar BOOL (bool literals, int/float 0/1, case-insensitive strings true/yes/on/1 and false/no/off/0). Python previously stored values verbatim, so heterogeneous lists failed downstream expression evaluation with 'List contains incompatible types', and homogeneous non-bool spellings silently produced wrongly-typed lists. Coerce each item to a canonical bool at the job-creation boundary: template defaults in _collect_defaults_2023_09 and submitted values (native list and JSON string) in _coerce_expr_param_value, gated exactly on LIST_BOOL. Invalid items keep 'Parameter <name>:' context. The boundary also covers environment-template merged defaults, which bypass model validators via model_copy. Behavior change: created-Job LIST[BOOL] values are now canonical booleans; previously-working homogeneous string lists (e.g. ["yes","no"]) now store and interpolate as true/false, matching the Rust reference implementation. LIST[FLOAT] integer items are intentionally left as-is (no spec-enumerated alternate spellings; the expression engine promotes int to float). Signed-off-by: Yongzhi Wei <276409147+wyongzhi@users.noreply.github.com>
Wrap template-default per-item coercion failures with the same Parameter <name>: prefix the submitted-value path uses. Add native-list adversarial coverage and a float-spelling case. Signed-off-by: Yongzhi Wei <276409147+wyongzhi@users.noreply.github.com>
Signed-off-by: Yongzhi Wei <276409147+wyongzhi@users.noreply.github.com>
Route only per-item coercion failures to the Parameter-name prefix. Move _coerce_bool_value to a version-agnostic module, dropping the inline imports. Pin template non-mutation and the None-default guard. Signed-off-by: Yongzhi Wei <276409147+wyongzhi@users.noreply.github.com>
jericht
approved these changes
Sep 3, 2026
seant-aws
approved these changes
Sep 3, 2026
Contributor
|
looks good ! |
wyongzhi
enabled auto-merge (squash)
September 3, 2026 23:07
This was referenced Sep 4, 2026
Closed
Merged
leongdl
added a commit
that referenced
this pull request
Sep 4, 2026
Rewrite the generated 0.11.9 block for users of the library. The generator emits one line per commit, so PR #341 rendered as six near-duplicate maintainer-facing bullets and PR #352 rendered twice. Each PR now gets one entry that states the observable behaviour change, the symptom it replaces, and for #341 the migration a consumer has to make. Commit hashes resolved to PR links. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl
added a commit
that referenced
this pull request
Sep 4, 2026
* chore(release): 0.11.9 Signed-off-by: client-software-ci <129794699+client-software-ci@users.noreply.github.com> * chore(release): clarify 0.11.9 release notes Rewrite the generated 0.11.9 block for users of the library. The generator emits one line per commit, so PR #341 rendered as six near-duplicate maintainer-facing bullets and PR #352 rendered twice. Each PR now gets one entry that states the observable behaviour change, the symptom it replaces, and for #341 the migration a consumer has to make. Commit hashes resolved to PR links. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> --------- Signed-off-by: client-software-ci <129794699+client-software-ci@users.noreply.github.com> Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> Co-authored-by: David Leong <116610336+leongdl@users.noreply.github.com>
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.
What was the problem/requirement? (What/Why)
Conformance fixtures
2.15--list-bool-param-{interpolation,runtime}(added by OpenJobDescription/openjd-specifications#158) pass on openjd-rs but fail on the Python CLI withList contains incompatible types: bool, string.Template Schemas §2.15 states each LIST[BOOL] item accepts the same values as scalar BOOL: bool literals, int/float
0/1, and the case-insensitive stringstrue/yes/on/1andfalse/no/off/0. The Python model validated those spellings at decode time but stored the list verbatim, so a heterogeneous list reached the expression engine un-normalized. Homogeneous non-bool spellings (e.g.["yes","off"],[1,0]) were worse: silently accepted as the wrong list type (ListString/ListInt), failing only when a boolean operator touched an element.What was the solution? (How)
Coerce each LIST[BOOL] item to a canonical bool at the job-creation boundary (
_create_job.pyonly; template models are not mutated):_collect_defaults_2023_09_coerce_expr_param_value, both native lists and JSON strings (the shared LIST JSON parse is unchanged for other list types)LIST_BOOL; invalid items keepParameter <name>:contextThe coercion reuses
_coerce_bool_value, the same function the scalar BOOL type and the decode-time item validation use, so the accepted value set cannot drift between the scalar and list types.Why convert at job creation, not at template decode (where openjd-rs converts): the difference is where the conversion can live. In Rust it lives in the type system:
BoolValuedeserializes"yes"straight into a bool, so an un-normalized value is unrepresentable. In pydantic it would live in validators, and validators can be bypassed: environment-template merges go throughmodel_copy, which skips them, so decode-time normalization leaks that path and the job-creation coercion is needed anyway. One site instead of two. It also keeps decoded templates round-tripping the author's spellings, a long-released behavior. Both implementations converge where conformance observes them: the created Job's values.LIST[FLOAT] integer items are intentionally not normalized: §2.14 enumerates no alternate spellings, no fixture pins it, and the expression engine promotes int to float. Noting it here rather than changing it silently.
What was the impact?
Created-Job LIST[BOOL] values are now canonical booleans. A previously-working homogeneous
["yes","no"]now stores and interpolates astrue/false, matching openjd-rs. Audited openjd-sessions, openjd-cli, and downstream consumers: none read the raw spellings.Template objects are unchanged: a decoded template's
defaultkeeps the author's original spellings.Testing
[[true]],[null],[2],[2.0], non-list JSON), empty list, environment-template merged defaults,Parameter <name>:context on per-item failures (JSON-level errors stay verbatim like other list types), template-object non-mutation and dump round-trip,default=Noneguard, and non-BOOL list guards (LIST[INT] JSON parsing, LIST[STRING]/LIST[PATH]/LIST[LIST[INT]] passthrough). Full suite: 5648 passed; ruff/black/mypy clean.run_openjd_cli_tests.py, full 2023-09 suite, released 0.11.6 vs this change): 1172 passed / 2 failed → 1174 passed / 0 failed. The only delta is the two 2.15 list-bool fixtures flipping to pass:By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.