From 6bb9e075bcd93beeb6b2da2e2f99d44e1b6ae5f8 Mon Sep 17 00:00:00 2001 From: David Leong Date: Tue, 1 Sep 2026 02:22:49 +0000 Subject: [PATCH 1/5] feat(model): Expose chunks_task_count_override on the v1 iterator Signed-off-by: David Leong --- rust-bindings/src/model/step_param_space.rs | 45 ++++- specs/python-model-interface.md | 20 +++ src/openjd/_openjd_rs.pyi | 17 +- test/openjd/model_v1/test_known_gaps.py | 78 ++++++++ .../model_v1/test_step_param_space_iter.py | 166 ++++++++++++++++++ 5 files changed, 320 insertions(+), 6 deletions(-) diff --git a/rust-bindings/src/model/step_param_space.rs b/rust-bindings/src/model/step_param_space.rs index c17cc468..f1d0b276 100644 --- a/rust-bindings/src/model/step_param_space.rs +++ b/rust-bindings/src/model/step_param_space.rs @@ -123,14 +123,42 @@ pub(crate) struct PyStepParameterSpaceIterator { /// every `NodeIterator` impl is `Send + Sync` (enforced at the /// trait bound in `openjd-model`). iter: Mutex, + /// The `chunks_task_count_override` this was constructed with, kept + /// so `__getitem__` can rebuild an equivalent iterator. Without it, + /// random access would silently fall back to the template's own + /// `defaultTaskCount` and disagree with iteration. + chunk_override: Option, } #[cfg_attr(feature = "stub-gen", gen_stub_pymethods)] #[pymethods] impl PyStepParameterSpaceIterator { + /// Construct an iterator over a step's parameter space. + /// + /// `chunks_task_count_override` overrides the `defaultTaskCount` of a + /// `CHUNK[INT]` parameter and turns adaptive chunking off, so a chunked space + /// can be walked at a caller-chosen granularity. Pass `1` to iterate individual + /// tasks. Ignored when the space has no chunked parameter, matching the + /// pure-Python reference. + /// + /// Without this, a statically chunked space could only be walked at the + /// template's own chunk size: `chunks_default_task_count` is settable for + /// adaptive spaces only. #[new] - #[pyo3(signature = (*, step=None, space=None))] - fn new(step: Option<&PyStep>, space: Option<&PyStepParameterSpace>) -> PyResult { + #[pyo3(signature = (*, step=None, space=None, chunks_task_count_override=None))] + fn new( + step: Option<&PyStep>, + space: Option<&PyStepParameterSpace>, + chunks_task_count_override: Option, + ) -> PyResult { + // Rejected rather than clamped: openjd-model applies `.max(1)` to the + // override, so 0 would silently mean 1. The `chunks_default_task_count` + // setter already refuses 0, and the two should not disagree. + if chunks_task_count_override == Some(0) { + return Err(pyo3::exceptions::PyValueError::new_err( + "chunks_task_count_override must be a positive integer.", + )); + } let ps = if let Some(s) = space { s.inner.clone() } else if let Some(st) = step { @@ -148,7 +176,9 @@ impl PyStepParameterSpaceIterator { combination: None, } }; - let iter = StepParameterSpaceIterator::new(&ps).map_err(model_err_to_py)?; + let iter = + StepParameterSpaceIterator::new_with_chunk_override(&ps, chunks_task_count_override) + .map_err(model_err_to_py)?; let len = iter.len(); let names = iter.names().clone(); Ok(Self { @@ -156,6 +186,7 @@ impl PyStepParameterSpaceIterator { len, names, iter: Mutex::new(iter), + chunk_override: chunks_task_count_override, }) } @@ -185,8 +216,12 @@ impl PyStepParameterSpaceIterator { index as usize }; // Random access uses a fresh iterator — don't disturb the - // persistent iter's cursor or its adaptive Arc. - let iter = StepParameterSpaceIterator::new(&self.space).map_err(model_err_to_py)?; + // persistent iter's cursor or its adaptive Arc. It must carry the + // same chunk override, or indexing would report chunks that + // iteration never yields. + let iter = + StepParameterSpaceIterator::new_with_chunk_override(&self.space, self.chunk_override) + .map_err(model_err_to_py)?; match iter.get(idx) { Some(params) => task_param_set_to_py(py, ¶ms), None => Err(pyo3::exceptions::PyIndexError::new_err( diff --git a/specs/python-model-interface.md b/specs/python-model-interface.md index aa8097c7..7a9d2320 100644 --- a/specs/python-model-interface.md +++ b/specs/python-model-interface.md @@ -1127,6 +1127,8 @@ from openjd.model._v1.job import StepParameterSpaceIterator it = StepParameterSpaceIterator(step=job.steps[0]) # or: it = StepParameterSpaceIterator(space=step.parameterSpace) +# or, to walk a chunked space one task at a time: +# it = StepParameterSpaceIterator(space=..., chunks_task_count_override=1) len(it) # total task count, e.g. 10 it[0] # {"Frame": 1} @@ -1150,6 +1152,24 @@ non-trivial state for chunked spaces). Indexing (``it[i]``) is unaffected by iteration position; ``__contains__`` is also non-mutating. +`chunks_task_count_override` replaces the `defaultTaskCount` of a +`CHUNK[INT]` parameter and turns adaptive chunking off, so a chunked +space can be walked at a granularity the caller picks. Pass `1` to +iterate individual tasks — a `1-20` range chunked five at a time then +yields `1-1`, `2-2`, … instead of `1-5`, `6-10`, …. It is ignored when +the space has no chunked parameter, matching the pure-Python reference. +Both iteration and indexing observe it, and `len()` counts the +overridden granularity. + +This is the only way to change the chunk size of a *static* chunked +space: the `chunks_default_task_count` setter accepts adaptive spaces +only, and raises `ValueError` otherwise. + +`chunks_task_count_override=0` raises `ValueError`. The Rust layer +clamps the override to at least 1, so 0 would silently mean 1, and the +`chunks_default_task_count` setter already rejects it. The pure-Python +reference does not validate this argument. + ### `StepDependencyGraph` Step dependency graph for topological ordering. diff --git a/src/openjd/_openjd_rs.pyi b/src/openjd/_openjd_rs.pyi index 205166a2..0487596f 100644 --- a/src/openjd/_openjd_rs.pyi +++ b/src/openjd/_openjd_rs.pyi @@ -2381,7 +2381,22 @@ class StepParameterSpaceIterator: *, step: typing.Optional[Step] = None, space: typing.Optional[StepParameterSpace] = None, - ) -> StepParameterSpaceIterator: ... + chunks_task_count_override: typing.Optional[builtins.int] = None, + ) -> StepParameterSpaceIterator: + r""" + Construct an iterator over a step's parameter space. + + `chunks_task_count_override` overrides the `defaultTaskCount` of a + `CHUNK[INT]` parameter and turns adaptive chunking off, so a chunked space + can be walked at a caller-chosen granularity. Pass `1` to iterate individual + tasks. Ignored when the space has no chunked parameter, matching the + pure-Python reference. + + Without this, a statically chunked space could only be walked at the + template's own chunk size: `chunks_default_task_count` is settable for + adaptive spaces only. + """ + def __len__(self) -> builtins.int: ... def __getitem__(self, index: builtins.int) -> dict: ... def __iter__(self) -> StepParameterSpaceIterator: diff --git a/test/openjd/model_v1/test_known_gaps.py b/test/openjd/model_v1/test_known_gaps.py index 3cb9c745..1c143d6d 100644 --- a/test/openjd/model_v1/test_known_gaps.py +++ b/test/openjd/model_v1/test_known_gaps.py @@ -19,6 +19,8 @@ from __future__ import annotations +from typing import Any + # ── Top-level package no longer leaks typing imports ── # # An earlier draft of ``openjd.model._v1`` imported ``Any``, @@ -41,3 +43,79 @@ def test_no_internal_imports_leak_at_top_level(name: str) -> None: import openjd.model._v1 as v1 assert not hasattr(v1, name), f"{name} leaks as a public attribute on openjd.model._v1" + + +# ── Chunked parameter spaces: two divergences from the v0 reference ── +# +# Found while adding `chunks_task_count_override` to +# `StepParameterSpaceIterator`. Neither is caused by that argument — both +# reproduce without it — so they are recorded here rather than fixed in +# passing. + + +def _chunked_step(constraint: str) -> Any: + from openjd.model._v1 import create_job, decode_job_template + + template = { + "specificationVersion": "jobtemplate-2023-09", + "name": "T", + "extensions": ["TASK_CHUNKING"], + "steps": [ + { + "name": "S", + "parameterSpace": { + "taskParameterDefinitions": [ + { + "name": "Frame", + "type": "CHUNK[INT]", + "range": "1-10", + "chunks": {"defaultTaskCount": 5, "rangeConstraint": constraint}, + } + ] + }, + "script": { + "actions": {"onRun": {"command": "echo", "args": ["{{Task.Param.Frame}}"]}} + }, + } + ], + } + job_template = decode_job_template(template=template, supported_extensions=["TASK_CHUNKING"]) + return create_job(job_template=job_template, job_parameter_values={}).steps[0] + + +@pytest.mark.xfail( + reason="v1 derives chunks_parameter_name and chunks_default_task_count from adaptive " + "detection, so both are None for a statically chunked space. v0 reports them for any " + "chunked space. See openjd-model step_param_space.rs: chunks_param_name and " + "adaptive_chunk_size are both built from adaptive_info.", + strict=True, +) +def test_chunk_metadata_is_reported_for_a_static_space() -> None: + """v0 returns ``"Frame"`` and ``5`` for this space. v1 returns ``None`` for both. + + Neither value is unknowable — both are in the template — so a consumer inspecting a + static chunked space through v1 cannot learn which parameter chunks, or at what size. + """ + from openjd.model._v1.job import StepParameterSpaceIterator + + it = StepParameterSpaceIterator(step=_chunked_step("CONTIGUOUS")) + assert it.chunks_adaptive is False + assert it.chunks_parameter_name == "Frame" + assert it.chunks_default_task_count == 5 + + +@pytest.mark.xfail( + reason="v1 refuses random access whenever the space needs sequential iteration, and " + "contiguous chunking always does. v0 supports indexing the same space.", + strict=True, +) +def test_a_contiguous_chunked_space_supports_indexing() -> None: + """v0 answers ``it[0]`` with ``1-5``. v1 raises ``IndexError``. + + ``len()`` works on this space, so the count is known; only ``get`` declines. + """ + from openjd.model._v1.job import StepParameterSpaceIterator + + it = StepParameterSpaceIterator(step=_chunked_step("CONTIGUOUS")) + assert len(it) == 2 + assert it[0]["Frame"].value == "1-5" diff --git a/test/openjd/model_v1/test_step_param_space_iter.py b/test/openjd/model_v1/test_step_param_space_iter.py index b87f292c..2e99087f 100644 --- a/test/openjd/model_v1/test_step_param_space_iter.py +++ b/test/openjd/model_v1/test_step_param_space_iter.py @@ -648,3 +648,169 @@ def test_explicit_chunk_value_round_trip(self) -> None: # caller wants to check whether a known chunk is in the space. existing = {"Frame": TaskParameterValue(type=TaskParameterType.CHUNK_INT, value="1-5")} assert existing in it + + +class TestChunksTaskCountOverride: + """``chunks_task_count_override`` re-chunks a space at the caller's granularity. + + Before this existed, a *statically* chunked space could only be walked at the + template's own ``defaultTaskCount``: the ``chunks_default_task_count`` setter + accepts adaptive spaces only. Consumers that store one task per chunk value — + the reason the pure-Python reference has this argument — had no way to expand a + static space through the binding. + """ + + @staticmethod + def _step(chunks: dict[str, Any], *, range: str = "1-10") -> Any: + """A single CHUNK[INT] step, built through decode + create_job so the test + exercises the same path a consumer hits at runtime.""" + t = decode_job_template( + template={ + "specificationVersion": "jobtemplate-2023-09", + "name": "T", + "extensions": ["TASK_CHUNKING"], + "steps": [ + { + "name": "S", + "parameterSpace": { + "taskParameterDefinitions": [ + { + "name": "Frame", + "type": "CHUNK[INT]", + "range": range, + "chunks": chunks, + } + ] + }, + "script": { + "actions": { + "onRun": {"command": "echo", "args": ["{{Task.Param.Frame}}"]} + } + }, + } + ], + }, + supported_extensions=["TASK_CHUNKING"], + ) + return create_job(job_template=t, job_parameter_values={}).steps[0] + + _STATIC = {"defaultTaskCount": 5, "rangeConstraint": "CONTIGUOUS"} + _ADAPTIVE = {"defaultTaskCount": 5, "targetRuntimeSeconds": 60, "rangeConstraint": "CONTIGUOUS"} + + @staticmethod + def _frames(it: Any) -> list: + return [params["Frame"].value for params in it] + + def test_static_space_without_override_yields_template_chunks(self) -> None: + """The baseline the override changes: 1-10 at 5 per chunk is two chunks.""" + it = StepParameterSpaceIterator(step=self._step(self._STATIC)) + assert self._frames(it) == ["1-5", "6-10"] + + def test_static_space_with_override_1_yields_individual_tasks(self) -> None: + """The case the argument exists for, and the one that was unreachable.""" + it = StepParameterSpaceIterator(step=self._step(self._STATIC), chunks_task_count_override=1) + assert self._frames(it) == [f"{n}-{n}" for n in range(1, 11)] + + def test_len_counts_the_overridden_granularity(self) -> None: + """``len()`` is the observable proof the override reached the space: the same + template counts 2 without it and 10 with it.""" + step = self._step(self._STATIC) + assert len(StepParameterSpaceIterator(step=step)) == 2 + assert len(StepParameterSpaceIterator(step=step, chunks_task_count_override=1)) == 10 + + def test_indexing_observes_the_override(self) -> None: + """``__getitem__`` builds a fresh iterator, so it has to carry the override too, + or indexing reports chunks iteration never yields. + + Uses NONCONTIGUOUS because random access needs a non-sequential space, and a + CONTIGUOUS chunked space is always sequential (see the test below). + """ + it = StepParameterSpaceIterator( + step=self._step({"defaultTaskCount": 5, "rangeConstraint": "NONCONTIGUOUS"}), + chunks_task_count_override=1, + ) + yielded = self._frames(it) + assert yielded == [str(n) for n in range(1, 11)] + # Without the override carried through, index 0 would be the template's first + # chunk, "1-5", and disagree with what iteration produced. + fresh = StepParameterSpaceIterator( + step=self._step({"defaultTaskCount": 5, "rangeConstraint": "NONCONTIGUOUS"}), + chunks_task_count_override=1, + ) + assert fresh[0]["Frame"].value == yielded[0] + assert fresh[9]["Frame"].value == yielded[-1] + assert fresh[-1]["Frame"].value == yielded[-1] + + def test_a_contiguous_space_refuses_indexing_with_or_without_the_override(self) -> None: + """Pre-existing behaviour the override does not change: openjd-model requires + sequential iteration for contiguous chunking, so ``get`` always declines. + Pinned here so a future change to random access is a deliberate one.""" + for override in (None, 1): + it = StepParameterSpaceIterator( + step=self._step(self._STATIC), chunks_task_count_override=override + ) + with pytest.raises(IndexError) as excinfo: + it[0] + assert str(excinfo.value) == "index out of range" + + def test_an_intermediate_override_regroups_the_chunks(self) -> None: + """Not just 1: any positive size regroups the space.""" + it = StepParameterSpaceIterator(step=self._step(self._STATIC), chunks_task_count_override=2) + assert self._frames(it) == ["1-2", "3-4", "5-6", "7-8", "9-10"] + + def test_override_turns_adaptive_chunking_off(self) -> None: + """Matches the pure-Python reference: supplying the override makes the + parameter static, which is also what makes ``len()`` answerable — an + adaptive space raises on ``len()`` because the count is not yet knowable.""" + adaptive = StepParameterSpaceIterator(step=self._step(self._ADAPTIVE)) + assert adaptive.chunks_adaptive is True + with pytest.raises(ValueError) as excinfo: + len(adaptive) + assert ( + str(excinfo.value) + == "Length is not available because the parameter space uses adaptive chunking." + ) + + overridden = StepParameterSpaceIterator( + step=self._step(self._ADAPTIVE), chunks_task_count_override=1 + ) + assert overridden.chunks_adaptive is False + assert len(overridden) == 10 + assert self._frames(overridden) == [f"{n}-{n}" for n in range(1, 11)] + + def test_override_is_ignored_when_the_space_has_no_chunked_parameter(self) -> None: + """Reference behaviour: the override only applies to a space that chunks.""" + space = StepParameterSpace( + taskParameterDefinitions={"Frame": {"type": "INT", "range": [1, 2, 3]}} + ) + it = StepParameterSpaceIterator(space=space, chunks_task_count_override=1) + assert [params["Frame"].value for params in it] == ["1", "2", "3"] + assert it.chunks_default_task_count is None + assert it.chunks_parameter_name is None + + def test_override_of_zero_is_rejected(self) -> None: + """openjd-model clamps the override to at least 1, so 0 would silently mean + 1. The ``chunks_default_task_count`` setter already refuses 0.""" + with pytest.raises(ValueError) as excinfo: + StepParameterSpaceIterator(step=self._step(self._STATIC), chunks_task_count_override=0) + assert str(excinfo.value) == "chunks_task_count_override must be a positive integer." + + def test_the_setter_still_refuses_a_static_space(self) -> None: + """The override does not replace the setter, and must not loosen it: the + setter mutates a live adaptive iterator, which a static space cannot do.""" + it = StepParameterSpaceIterator(step=self._step(self._STATIC)) + with pytest.raises(ValueError) as excinfo: + it.chunks_default_task_count = 1 + assert str(excinfo.value) == ( + "The parameter space does not use adaptive chunking, " + "so cannot modify chunks_default_task_count." + ) + + def test_yielded_values_round_trip_through_contains(self) -> None: + """An overridden chunk must still satisfy containment, so a consumer can + validate a task it was handed.""" + step = self._step(self._STATIC) + it = StepParameterSpaceIterator(step=step, chunks_task_count_override=1) + fresh = StepParameterSpaceIterator(step=step, chunks_task_count_override=1) + for params in list(it): + assert params in fresh From 6b3d9beca7fa97836d99852918968e8472f7612b Mon Sep 17 00:00:00 2001 From: David Leong Date: Tue, 1 Sep 2026 02:29:38 +0000 Subject: [PATCH 2/5] test(model): Bind the discarded subscript so CodeQL sees an effect Signed-off-by: David Leong --- test/openjd/model_v1/test_step_param_space_iter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/openjd/model_v1/test_step_param_space_iter.py b/test/openjd/model_v1/test_step_param_space_iter.py index 2e99087f..a7d786db 100644 --- a/test/openjd/model_v1/test_step_param_space_iter.py +++ b/test/openjd/model_v1/test_step_param_space_iter.py @@ -750,7 +750,7 @@ def test_a_contiguous_space_refuses_indexing_with_or_without_the_override(self) step=self._step(self._STATIC), chunks_task_count_override=override ) with pytest.raises(IndexError) as excinfo: - it[0] + _ = it[0] assert str(excinfo.value) == "index out of range" def test_an_intermediate_override_regroups_the_chunks(self) -> None: From e15203009b02d0f38b1c557e9209c667cbef0ab3 Mon Sep 17 00:00:00 2001 From: David Leong Date: Tue, 1 Sep 2026 02:53:02 +0000 Subject: [PATCH 3/5] fix(model): Report every non-positive chunk override as ValueError Signed-off-by: David Leong --- rust-bindings/src/model/step_param_space.rs | 26 ++++++++++++------- specs/python-model-interface.md | 8 ++++-- .../model_v1/test_step_param_space_iter.py | 16 +++++++++--- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/rust-bindings/src/model/step_param_space.rs b/rust-bindings/src/model/step_param_space.rs index f1d0b276..3ab321c4 100644 --- a/rust-bindings/src/model/step_param_space.rs +++ b/rust-bindings/src/model/step_param_space.rs @@ -149,16 +149,24 @@ impl PyStepParameterSpaceIterator { fn new( step: Option<&PyStep>, space: Option<&PyStepParameterSpace>, - chunks_task_count_override: Option, + chunks_task_count_override: Option, ) -> PyResult { - // Rejected rather than clamped: openjd-model applies `.max(1)` to the - // override, so 0 would silently mean 1. The `chunks_default_task_count` - // setter already refuses 0, and the two should not disagree. - if chunks_task_count_override == Some(0) { - return Err(pyo3::exceptions::PyValueError::new_err( - "chunks_task_count_override must be a positive integer.", - )); - } + // Taken as i64, not usize, so a negative value reaches this check instead of + // failing pyo3's unsigned extraction with OverflowError. Every non-positive + // value should report the same ValueError the message and the spec promise. + // + // Rejected rather than clamped: openjd-model applies `.max(1)` to the override, + // so 0 would silently mean 1. The `chunks_default_task_count` setter already + // refuses 0, and the two should not disagree. + let chunks_task_count_override: Option = match chunks_task_count_override { + Some(n) if n <= 0 => { + return Err(pyo3::exceptions::PyValueError::new_err( + "chunks_task_count_override must be a positive integer.", + )); + } + Some(n) => Some(n as usize), + None => None, + }; let ps = if let Some(s) = space { s.inner.clone() } else if let Some(st) = step { diff --git a/specs/python-model-interface.md b/specs/python-model-interface.md index 7a9d2320..6dc44b2f 100644 --- a/specs/python-model-interface.md +++ b/specs/python-model-interface.md @@ -1158,8 +1158,12 @@ space can be walked at a granularity the caller picks. Pass `1` to iterate individual tasks — a `1-20` range chunked five at a time then yields `1-1`, `2-2`, … instead of `1-5`, `6-10`, …. It is ignored when the space has no chunked parameter, matching the pure-Python reference. -Both iteration and indexing observe it, and `len()` counts the -overridden granularity. +Iteration observes it, and `len()` counts the overridden granularity. + +Indexing observes it too, but only for a space that supports random +access. A `CONTIGUOUS` chunked space requires sequential iteration, so +`it[i]` raises `IndexError` for any index — with or without the +override — even though `len(it)` reports a count. This is the only way to change the chunk size of a *static* chunked space: the `chunks_default_task_count` setter accepts adaptive spaces diff --git a/test/openjd/model_v1/test_step_param_space_iter.py b/test/openjd/model_v1/test_step_param_space_iter.py index a7d786db..e2e9e7ef 100644 --- a/test/openjd/model_v1/test_step_param_space_iter.py +++ b/test/openjd/model_v1/test_step_param_space_iter.py @@ -788,11 +788,19 @@ def test_override_is_ignored_when_the_space_has_no_chunked_parameter(self) -> No assert it.chunks_default_task_count is None assert it.chunks_parameter_name is None - def test_override_of_zero_is_rejected(self) -> None: - """openjd-model clamps the override to at least 1, so 0 would silently mean - 1. The ``chunks_default_task_count`` setter already refuses 0.""" + @pytest.mark.parametrize("override", [0, -1, -5]) + def test_a_non_positive_override_is_rejected_as_a_value_error(self, override: int) -> None: + """openjd-model clamps the override to at least 1, so 0 would silently mean 1, and + the ``chunks_default_task_count`` setter already refuses 0. + + Negatives report the same ``ValueError`` rather than the ``OverflowError`` an + unsigned extraction would raise, so one ``except ValueError`` covers every + non-positive input. + """ with pytest.raises(ValueError) as excinfo: - StepParameterSpaceIterator(step=self._step(self._STATIC), chunks_task_count_override=0) + StepParameterSpaceIterator( + step=self._step(self._STATIC), chunks_task_count_override=override + ) assert str(excinfo.value) == "chunks_task_count_override must be a positive integer." def test_the_setter_still_refuses_a_static_space(self) -> None: From 64858251853f5816ea906cf174a8fd41e69eabab Mon Sep 17 00:00:00 2001 From: David Leong Date: Tue, 1 Sep 2026 03:03:39 +0000 Subject: [PATCH 4/5] test(model): Pin chunk-metadata divergence across every non-adaptive case Signed-off-by: David Leong --- specs/python-model-interface.md | 31 ++++++++----- test/openjd/model_v1/test_known_gaps.py | 60 +++++++++++++++++++++---- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/specs/python-model-interface.md b/specs/python-model-interface.md index 6dc44b2f..2fa53e9c 100644 --- a/specs/python-model-interface.md +++ b/specs/python-model-interface.md @@ -1160,20 +1160,31 @@ yields `1-1`, `2-2`, … instead of `1-5`, `6-10`, …. It is ignored when the space has no chunked parameter, matching the pure-Python reference. Iteration observes it, and `len()` counts the overridden granularity. -Indexing observes it too, but only for a space that supports random -access. A `CONTIGUOUS` chunked space requires sequential iteration, so -`it[i]` raises `IndexError` for any index — with or without the -override — even though `len(it)` reports a count. +It is currently the only way to change the chunk size of a *static* +chunked space: the `chunks_default_task_count` setter accepts adaptive +spaces only, and raises `ValueError` otherwise. -This is the only way to change the chunk size of a *static* chunked -space: the `chunks_default_task_count` setter accepts adaptive spaces -only, and raises `ValueError` otherwise. - -`chunks_task_count_override=0` raises `ValueError`. The Rust layer -clamps the override to at least 1, so 0 would silently mean 1, and the +Any non-positive value raises `ValueError`, so one `except ValueError` +covers `0` and negatives alike. The Rust layer clamps the override to at +least 1, so 0 would otherwise silently mean 1, and the `chunks_default_task_count` setter already rejects it. The pure-Python reference does not validate this argument. +Two current-implementation limitations, both divergences from the v0 +reference rather than intended behaviour. Each has a `strict` xfail in +`test/openjd/model_v1/test_known_gaps.py`, so clearing either will fail +CI until this text is updated with it. + +- Indexing observes the override only for a space that supports random + access. A `CONTIGUOUS` chunked space requires sequential iteration, so + `it[i]` raises `IndexError` for any index — with or without the + override — even though `len(it)` reports a count. See + `test_a_contiguous_chunked_space_supports_indexing`. +- `chunks_parameter_name` and `chunks_default_task_count` both return + `None` once the space is non-adaptive, which supplying the override + makes it. v0 reports the parameter name and the override value. See + `test_chunk_metadata_is_reported_for_a_non_adaptive_space`. + ### `StepDependencyGraph` Step dependency graph for topological ordering. diff --git a/test/openjd/model_v1/test_known_gaps.py b/test/openjd/model_v1/test_known_gaps.py index 1c143d6d..bbc50c6b 100644 --- a/test/openjd/model_v1/test_known_gaps.py +++ b/test/openjd/model_v1/test_known_gaps.py @@ -85,23 +85,65 @@ def _chunked_step(constraint: str) -> Any: @pytest.mark.xfail( reason="v1 derives chunks_parameter_name and chunks_default_task_count from adaptive " - "detection, so both are None for a statically chunked space. v0 reports them for any " - "chunked space. See openjd-model step_param_space.rs: chunks_param_name and " + "detection, so both are None for any non-adaptive chunked space. v0 reports them for " + "any chunked space. See openjd-model step_param_space.rs: chunks_param_name and " "adaptive_chunk_size are both built from adaptive_info.", strict=True, ) -def test_chunk_metadata_is_reported_for_a_static_space() -> None: - """v0 returns ``"Frame"`` and ``5`` for this space. v1 returns ``None`` for both. - - Neither value is unknowable — both are in the template — so a consumer inspecting a - static chunked space through v1 cannot learn which parameter chunks, or at what size. +@pytest.mark.parametrize( + "chunks,override,expected_count", + [ + # Statically chunked: no override involved, v0 reports the template's size. + ({"defaultTaskCount": 5, "rangeConstraint": "CONTIGUOUS"}, None, 5), + # Statically chunked, re-chunked by the override. + ({"defaultTaskCount": 5, "rangeConstraint": "CONTIGUOUS"}, 1, 1), + # Adaptive, turned static by the override. v0 reports the override as the size. + ( + {"defaultTaskCount": 5, "targetRuntimeSeconds": 60, "rangeConstraint": "CONTIGUOUS"}, + 1, + 1, + ), + ], + ids=["static", "static-overridden", "adaptive-overridden"], +) +def test_chunk_metadata_is_reported_for_a_non_adaptive_space( + chunks: dict, override: int | None, expected_count: int +) -> None: + """v0 returns ``"Frame"`` and the chunk size for each of these. v1 returns ``None``. + + Neither value is unknowable — both are in the template, or are the override the caller + just passed — so a consumer inspecting a non-adaptive chunked space through v1 cannot + learn which parameter chunks, or at what size. One root cause, three ways to reach it: + anything that leaves the space non-adaptive drops both getters. """ + from openjd.model._v1 import create_job, decode_job_template from openjd.model._v1.job import StepParameterSpaceIterator - it = StepParameterSpaceIterator(step=_chunked_step("CONTIGUOUS")) + template = { + "specificationVersion": "jobtemplate-2023-09", + "name": "T", + "extensions": ["TASK_CHUNKING"], + "steps": [ + { + "name": "S", + "parameterSpace": { + "taskParameterDefinitions": [ + {"name": "Frame", "type": "CHUNK[INT]", "range": "1-10", "chunks": chunks} + ] + }, + "script": { + "actions": {"onRun": {"command": "echo", "args": ["{{Task.Param.Frame}}"]}} + }, + } + ], + } + job_template = decode_job_template(template=template, supported_extensions=["TASK_CHUNKING"]) + step = create_job(job_template=job_template, job_parameter_values={}).steps[0] + + it = StepParameterSpaceIterator(step=step, chunks_task_count_override=override) assert it.chunks_adaptive is False assert it.chunks_parameter_name == "Frame" - assert it.chunks_default_task_count == 5 + assert it.chunks_default_task_count == expected_count @pytest.mark.xfail( From 99fdecff7aed4d133bcaa1d676667893ed7d13cb Mon Sep 17 00:00:00 2001 From: David Leong Date: Tue, 1 Sep 2026 05:49:58 +0000 Subject: [PATCH 5/5] docs: narrow the override wording and pin the eager validation Signed-off-by: David Leong --- rust-bindings/src/model/step_param_space.rs | 4 +++- specs/python-model-interface.md | 8 +++++++- src/openjd/_openjd_rs.pyi | 3 ++- .../model_v1/test_step_param_space_iter.py | 17 ++++++++++++++++- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/rust-bindings/src/model/step_param_space.rs b/rust-bindings/src/model/step_param_space.rs index 3ab321c4..a764c807 100644 --- a/rust-bindings/src/model/step_param_space.rs +++ b/rust-bindings/src/model/step_param_space.rs @@ -139,7 +139,9 @@ impl PyStepParameterSpaceIterator { /// `CHUNK[INT]` parameter and turns adaptive chunking off, so a chunked space /// can be walked at a caller-chosen granularity. Pass `1` to iterate individual /// tasks. Ignored when the space has no chunked parameter, matching the - /// pure-Python reference. + /// pure-Python reference — though a non-positive value is still rejected in + /// that case, since validating an argument is cheaper to reason about than + /// silently discarding a bad one. /// /// Without this, a statically chunked space could only be walked at the /// template's own chunk size: `chunks_default_task_count` is settable for diff --git a/specs/python-model-interface.md b/specs/python-model-interface.md index 2fa53e9c..3150975b 100644 --- a/specs/python-model-interface.md +++ b/specs/python-model-interface.md @@ -1157,9 +1157,15 @@ non-mutating. space can be walked at a granularity the caller picks. Pass `1` to iterate individual tasks — a `1-20` range chunked five at a time then yields `1-1`, `2-2`, … instead of `1-5`, `6-10`, …. It is ignored when -the space has no chunked parameter, matching the pure-Python reference. +the space has no chunked parameter, matching the pure-Python reference, +although a non-positive value is still rejected in that case. Iteration observes it, and `len()` counts the overridden granularity. +The rendered form of a chunk depends on `rangeConstraint`, which matters +because these are strings a consumer parses and may feed back through +`__contains__`. A single-task chunk is `"1-1"` under `CONTIGUOUS` and a +bare `"1"` under `NONCONTIGUOUS`. + It is currently the only way to change the chunk size of a *static* chunked space: the `chunks_default_task_count` setter accepts adaptive spaces only, and raises `ValueError` otherwise. diff --git a/src/openjd/_openjd_rs.pyi b/src/openjd/_openjd_rs.pyi index 0487596f..c05192bb 100644 --- a/src/openjd/_openjd_rs.pyi +++ b/src/openjd/_openjd_rs.pyi @@ -2390,7 +2390,8 @@ class StepParameterSpaceIterator: `CHUNK[INT]` parameter and turns adaptive chunking off, so a chunked space can be walked at a caller-chosen granularity. Pass `1` to iterate individual tasks. Ignored when the space has no chunked parameter, matching the - pure-Python reference. + pure-Python reference -- though a non-positive value is still rejected in + that case. Without this, a statically chunked space could only be walked at the template's own chunk size: `chunks_default_task_count` is settable for diff --git a/test/openjd/model_v1/test_step_param_space_iter.py b/test/openjd/model_v1/test_step_param_space_iter.py index e2e9e7ef..acb233a0 100644 --- a/test/openjd/model_v1/test_step_param_space_iter.py +++ b/test/openjd/model_v1/test_step_param_space_iter.py @@ -744,7 +744,11 @@ def test_indexing_observes_the_override(self) -> None: def test_a_contiguous_space_refuses_indexing_with_or_without_the_override(self) -> None: """Pre-existing behaviour the override does not change: openjd-model requires sequential iteration for contiguous chunking, so ``get`` always declines. - Pinned here so a future change to random access is a deliberate one.""" + Pinned here so a future change to random access is a deliberate one. Its + counterpart is ``test_known_gaps.py::test_a_contiguous_chunked_space_supports_indexing``, + a strict xfail asserting the opposite: closing that gap fails there as an xpass + *and* here as a hard assertion, so both move together, along with the + limitation noted in ``specs/python-model-interface.md``.""" for override in (None, 1): it = StepParameterSpaceIterator( step=self._step(self._STATIC), chunks_task_count_override=override @@ -778,6 +782,17 @@ def test_override_turns_adaptive_chunking_off(self) -> None: assert len(overridden) == 10 assert self._frames(overridden) == [f"{n}-{n}" for n in range(1, 11)] + def test_a_non_positive_override_is_rejected_even_when_it_would_be_ignored(self) -> None: + """The validation runs before the space is inspected, so an unchunked space still + rejects 0 rather than silently discarding it. The reference does not validate at + all; this is the documented divergence, pinned so it stays deliberate.""" + space = StepParameterSpace( + taskParameterDefinitions={"Frame": {"type": "INT", "range": [1, 2, 3]}} + ) + with pytest.raises(ValueError) as excinfo: + StepParameterSpaceIterator(space=space, chunks_task_count_override=0) + assert str(excinfo.value) == "chunks_task_count_override must be a positive integer." + def test_override_is_ignored_when_the_space_has_no_chunked_parameter(self) -> None: """Reference behaviour: the override only applies to a space that chunks.""" space = StepParameterSpace(