fix(model): do not cap <IntRangeExpr> expansion at the list-form limit

[leongdl]

What

max_task_param_range_len (1024) is enforced against the expansion of an <IntRangeExpr>. It should apply to the list forms only. This drops the check on the expression form in both the template-validation and job-creation paths, and keeps every list-form check untouched.

Why

§3.4 states the 1024 cap for the list forms only:

• <IntRangeList> — §3.4.1.1, item 4
• <FloatRangeList> — §3.4.1.2
• <StringRangeList> — §3.4.1.3

§3.4.1.1.1 <IntRangeExpr> states no element cap. Its only stated constraint is that no two ranges may overlap, and its stated purpose is "a succinct way to describe a frame range" — so capping the expansion at 1024 rejects the form's primary use case. 1-5000 is a perfectly ordinary frame range.

The cap also pre-empts the host service's own limits. A service that lets a customer raise tasks-per-step above 1024 cannot honour that quota, because the model rejects the template before the service ever sees the task count. A host that wants to bound fan-out already has CallerLimits::max_task_count, which is the right lever: CallerLimits is documented as only ever adding restrictions, so it can tighten without the model having to guess a ceiling.

Concretely, this made a 10,000-task-per-step quota unreachable via a single wide range.

Provenance

The check has been present since the initial port (1e6752c), which predates any PR in this repo, so it has not previously been reviewed on its own. It was subsequently mirrored into openjd-model-for-python by #318 and shipped in 0.11.1, which is where it began breaking real templates.

Changes

• template/validate_v2023_09/structure.rs — the IntRange::Expression arm now validates grammar only. Invalid expressions are still rejected; valid ones are no longer length-checked.
• job/create_job/ranges.rs — dropped the length check from the ExprValue::RangeExpr branch and from the resolved-format-string RangeExpr branch. The List branches keep theirs (resolve_int_range, resolve_float_range, resolve_string_range).
• template/validate_v2023_09/mod.rs — doc comment on max_task_param_range_len scoping it to the list forms and pointing hosts at CallerLimits::max_task_count.
• specs/model/parameters.md — same scoping note beside the limits table.

Tests

New integration tests:

• test_parameter_space::int_range_expression_expansion_is_not_capped — 1024, 1025, 5000 and 1-100000:2 accepted, asserting the full expansion length at both the template and instantiation layers
• test_parameter_space::int_range_expression_grammar_is_still_validated — malformed expressions still produce range expression error
• test_parameter_space::int_range_list_is_still_capped / string_range_list_is_still_capped — 1025-element lists still rejected with range exceeds 1024 elements
• test_chunk_int::range_expression_expansion_is_not_capped / range_list_is_still_capped — same split for CHUNK[INT]

cargo test --workspace is green (0 failures). cargo clippy -p openjd-model --all-targets and cargo fmt --all -- --check are clean.

Related

The equivalent Python fix is openjd-model-for-python#327.

[github-actions]

Dropping the cap here removes the only bound on an IntRangeExpr expansion, and there is at least one path that then materializes the full expansion into memory.

make_chunk_node in step_param_space.rs eagerly collects when adaptive chunking is selected:

// crates/openjd-model/src/job/step_param_space.rs:1720-1722
let values: Vec<i64> = match range {
    job::TaskParamRange::List(v) => v.clone(),
    job::TaskParamRange::RangeExpr(r) => r.iter().collect(),   // <-- full expansion
};

Adaptive chunking is selected whenever a CHUNK[INT] parameter sets chunks.targetRuntimeSeconds > 0 (new_inner, ~line 1307), and AdaptiveChunkNode.values is a Vec<i64>. So a template with

type: CHUNK[INT]
range: "1-1000000000"
chunks: { defaultTaskCount: 10, targetRuntimeSeconds: 60 }

now allocates ~8 GB before any limit is consulted. RangeExpr values are bounded only by MAX_RANGE_VALUE_MAGNITUDE (2^62), so 1-4611686018427387903 is also accepted by the parser.

The stated backstop does not cover this:

• CallerLimits::max_task_count is Option<u64> and defaults to None, so in the default configuration there is no bound at all.
• Even when it is set, the check in create_job (mod.rs:115) runs after instantiate_step for all steps, and it builds the iterator with new_with_chunk_override(ps, Some(1)) — which deliberately skips adaptive_info. So the count check never exercises the allocating path, and cannot pre-empt a consumer that later iterates the space for real.

The lazy RangeExprNode / ContiguousChunkNode paths are genuinely index-based and fine (the test_truly_lazy_trillion_element_space test covers those). The adaptive path is the outlier.

Suggest either bounding the eager collect in make_chunk_node (return a ModelError instead of allocating), or keeping a separate expansion limit that is decoupled from the spec-derived max_task_param_range_len so the §3.4.1.1.1 argument in this PR still holds.

[github-actions]

This doc comment asserts CallerLimits::max_task_count is a sufficient substitute for the removed cap. It is not, for two reasons beyond the eager-allocation path I flagged in ranges.rs:

1. It is opt-in and defaults to off. CallerLimits::max_task_count is Option<u64> (types.rs:427) and CallerLimits::default() leaves it None — asserted by the crate's own test at test_caller_limits.rs:581. Every caller that does not explicitly set it (including all the decode_ok/check_err helpers the new tests use) now has no bound on range-expression fan-out at all, where before it had 1024.

2. It does not gate the O(num_chunks) containment scan. StaticChunkNode::validate_containment is a linear scan that rebuilds a RangeExpr per chunk:

// step_param_space.rs:859
if (0..self.num_chunks).any(|i| self.chunk_range_expr(i) == *r) {

ContiguousChunkNode::validate_containment (~line 425) is the same shape — it iterates every chunk looking for a match. num_chunks comes from total_len.div_ceil(default_task_count), so range: "1-1000000000" with defaultTaskCount: 10 yields 10^8 iterations, each doing a format! + parse::<RangeExpr>().

This is reachable from openjd-cli: execute_explicit_tasks calls iter.validate_containment(&values) per user-supplied task-parameter set (crates/openjd-cli/src/run/execution.rs:439), and a non-matching value costs the full scan. Under the old cap num_chunks <= 1024, so this was bounded; it no longer is, and max_task_count is not consulted on this path at all.

If the §3.4.1.1.1 reading is right that the spec does not cap the expansion, then these two containment scans need to become index-arithmetic lookups (locate the chunk containing r's first value, then compare) rather than linear searches — otherwise the cap removal converts a validation rejection into a CPU-exhaustion vector.

[github-actions]

"1-100000:2" in this list is the interesting case, and it points at a third place the removed cap was load-bearing.

count_contiguous_chunks_from_sub_ranges documents itself as O(R) where R is the number of sub-ranges, "not the number of values". That is true only for step == 1 sub-ranges. The else branch expands per value:

// step_param_space.rs:359-364
} else {
    // Step > 1: each value is isolated (has gaps between them).
    let count = sr.len();
    for idx in 0..count {                       // <-- O(values), not O(sub-ranges)
        let val = sr.get(idx).expect("index within sub-range bounds");

This runs at node-construction time — ContiguousChunkNode::new calls count_contiguous_chunks_for_range eagerly to cache num_chunks — so it is not deferred by the lazy-iteration design that makes RangeExprNode safe.

Under the old 1024 cap this loop was bounded. With the cap removed, range: "1-1000000000:2" + rangeConstraint: CONTIGUOUS is 5x10^8 iterations of sr.get() before a single task is produced, and 1-4611686018427387903:2 is accepted by the RangeExpr parser (bounded only by MAX_RANGE_VALUE_MAGNITUDE).

Note this test only reaches decode_job_template, which validates the template but never constructs a StepParameterSpaceIterator — so it will pass without exercising the path. A test that goes through create_job + iterator construction with a stepped CONTIGUOUS range would surface it.

The counting for stepped sub-ranges is closed-form (each value is its own interval, so it contributes ceil(1/dtc) == 1 chunk unless it is adjacent to the previous interval, which for step >= 2 it never is). Replacing the loop with arithmetic would restore the documented O(R) and remove the need for a cap here.

[github-actions]

This PR removes the cap at two sites, but the new tests only cover one of them.

decode_ok here calls decode_job_template (helper at line 29), which runs the validation pipeline only — it never calls create_job. So these tests exercise the structure.rs::validate_task_param_range removal but not the ranges.rs::resolve_int_range removal, which is the site that actually produces the TaskParamRange::RangeExpr a consumer will later iterate.

Two consequences:

1. The ranges.rs change is unverified by this PR. Notably resolve_int_range has two paths that were capped — the typed-eval ExprValue::RangeExpr(r) arm (line 214) and the string-resolution fallback (line 251) — and neither is reached by a decode-only test. A create_job-based test would also cover the "1-{{Param.Count}}" format-string case, which decode skips entirely because of the !raw.contains("{{") guard in structure.rs.

2. The list-form caps that these tests assert still apply (int_range_list_is_still_capped, string_range_list_is_still_capped) are likewise only proving the structure.rs cap fires. The parallel caps in ranges.rs (lines 190, 229, 324, 377) are what protect a caller that constructs a Job directly, and they remain untested here.

Suggest adding at least one test that goes through create_job and then StepParameterSpaceIterator::new, in the style of lazy_param_space_range_expr_within_limit in test_step_param_space_iter.rs — that is the path where an uncapped expansion actually costs something.

Separately: that existing test is now named ..._within_limit and its explanatory comment about max_task_param_range_len was deleted in this PR, leaving a name that references a limit no longer applied to range expressions. Worth renaming.

[seant-aws]

good catch

[github-actions]

This test is vacuous with respect to the change: it would pass unmodified on the base commit.

The CHUNK_INT arm of validate_task_param_range never had a length cap on the expression form — the diff leaves it untouched, and in the current file (structure.rs:1155-1166) it only does a grammar parse:

IntRange::Expression(expr) => {
    if !expr.raw().contains("{{") {
        if let Err(e) = expr.raw().parse::<openjd_expr::RangeExpr>() {

Only the INT arm was capped and only that arm was edited. Since decode_ok stops at decode_job_template, "1-1025" through a CHUNK[INT] parameter was already accepted before this PR.

That makes this a no-op regression guard. To actually pin the CHUNK[INT] behavior the cap removal affects, the test needs to reach create_job (which calls resolve_int_range, the site this PR edits) and construct the iterator.

[godobyte]

Please check the conf tests failure in CI

[leongdl]

The failing conformance case (2023-09/base/job_templates/3.4--too-many-range-items.invalid.yaml) asserts that range: "1-1025" — an <IntRangeExpr> — is invalid. §3.4.1.1.1 places no element cap on that form; the 1024 maximum belongs to <IntRangeList> (§3.4.1.1 item 4). The reference Python implementation applied the cap to the list forms only through 0.11.0, so the suite has been asserting behavior neither the spec text nor the reference implementation required.

Corrected in OpenJobDescription/openjd-specifications#172, which moves that test to list form, adds a valid wide-expression case, and states the rule in §3.4.1.1.1. Against a build of this branch the amended suite reports 1160 passed / 0 failed. This PR should merge after #172.