Skip to content

fix: coerce LIST[BOOL] items per RFC 0007 §2.15 - #352

Merged
wyongzhi merged 4 commits into
OpenJobDescription:mainlinefrom
wyongzhi:fix/list-bool-item-coercion
Sep 3, 2026
Merged

fix: coerce LIST[BOOL] items per RFC 0007 §2.15#352
wyongzhi merged 4 commits into
OpenJobDescription:mainlinefrom
wyongzhi:fix/list-bool-item-coercion

Conversation

@wyongzhi

@wyongzhi wyongzhi commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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 with List 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 strings true/yes/on/1 and false/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.py only; template models are not mutated):

YAML template
   |  decode (pydantic validates spellings, stores them verbatim; unchanged)
   v
Template object    default = ["yes", 0, True]    <- keeps the author's spellings (pinned by test)
   |
   |  create_job()  * conversion happens at this gate
   v
Job object         value = [True, False, True]   <- every reader sees canonical bools
   |
   v
Expression engine / sessions / cli               <- untouched, fixed for free
  • template defaults: _collect_defaults_2023_09
  • submitted values: _coerce_expr_param_value, both native lists and JSON strings (the shared LIST JSON parse is unchanged for other list types)
  • gated exactly on LIST_BOOL; invalid items keep Parameter <name>: context

The 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: BoolValue deserializes "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 through model_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 as true/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 default keeps the author's original spellings.

Testing

  • 31 new unit tests: mixed/homogeneous/mixed-case spellings, native and JSON submitted values, adversarial items via both the JSON and native-list paths ([[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=None guard, 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.
  • Conformance A/B (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:
240,241c240,241
<   ✗ 2.15--list-bool-param-interpolation
<   ✗ 2.15--list-bool-param-runtime
---
>   ✓ 2.15--list-bool-param-interpolation
>   ✓ 2.15--list-bool-param-runtime

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

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>
@wyongzhi
wyongzhi requested a review from a team as a code owner September 3, 2026 21:11
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>
@seant-aws

Copy link
Copy Markdown
Contributor

looks good !

@wyongzhi
wyongzhi enabled auto-merge (squash) September 3, 2026 23:07
@wyongzhi
wyongzhi merged commit 105dff2 into OpenJobDescription:mainline Sep 3, 2026
36 of 49 checks passed
@wyongzhi
wyongzhi deleted the fix/list-bool-item-coercion branch September 3, 2026 23:22
This was referenced Sep 4, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants