Skip to content

fix: Keep the trailing zeros of a floatstring range element #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leongdl wants to merge 3 commits into
base: mainline
Choose a base branch
from
Open

fix: Keep the trailing zeros of a floatstring range element #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af89643
fix: Keep the trailing zeros of a floatstring range element
leongdl Sep 1, 2026
0708da8
fix: trim a floatstring range element and unsign a zero
leongdl Sep 1, 2026
41bf56c
fix: decide a zero spelling from the text, not the parse
leongdl Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 41 additions & 79 deletions src/openjd/model/v2023_09/_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,74 +1245,44 @@ def _validate_target_runtime_seconds(cls, value: Any, info: ValidationInfo) -> A
return validate_int_fmtstring_field(value, ge=0, context=context)


# '02.50' -> '2.50', '007' -> '7', '000' -> '0'. The lookahead leaves the last
# digit, so '0.50' keeps the zero that is its integer part.
_REDUNDANT_LEADING_ZEROS = re.compile(r"^([+-]?)0+(?=[0-9])")
Comment thread
leongdl marked this conversation as resolved.

# An all-zero mantissa, whatever the exponent: '0.00', '-0.0' and '0e5' spell zero,
# '1e-400' does not. Mirrors openjd_expr::value::text_spells_zero.
_SPELLS_ZERO = re.compile(r"^[+-]?[0.]*[0][0.]*(?:[eE][+-]?[0-9]+)?$")

@github-actions github-actions Bot Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_SPELLS_ZERO backtracks quadratically on unbounded input, which reintroduces — in a different form — the resource bound the PR removes.

The mantissa is written as three overlapping pieces, [0.]*[0][0.]*, so for an input the pattern fails on, the engine tries every split point: first star gives back one char, second star re-tries every remaining length, $ fails, repeat. That is O(n²) with no early exit.

A failing input is easy to construct and is exactly the shape the PR now accepts:

  • "0." + "0"*n + "1" — float() parses it (underflows to 0.0 for large n), _REDUNDANT_LEADING_ZEROS does not shorten it (its (?=[0-9]) lookahead sees the ., so it does not match at all), and then _spells_zero runs the quadratic scan and ultimately returns "no".

The input is not length-bounded when this runs. _normalized_range_element is called from a mode="before" validator, so it sees the raw element before TaskRangeList's TaskParameterStringValueAsJob 1024-char constraint is applied — and on the template side the element type is TaskParameterStringValue, whose own comment says "No maximum length. The max string length is enforced as a TaskParameterStringValueAsJob type after the template has been instantiated into a Job." So a 100 KB element reaches the regex intact, at ~5·10⁹ steps, and a range may hold 1024 elements.

The old code could not do this: Decimal(elem) and format(value, "f") are linear in the text, and the exponent guard bounded the one super-linear step. The new test "0." + "0"*400 + "1" exercises this path but at n=400 the cost is invisible, so CI will not surface it.

The check does not need a backtracking pattern — it is "the mantissa is made only of 0 and ., and contains at least one 0", which a linear scan states directly:

def _spells_zero(text: str) -> bool:
    mantissa = re.sub(r"[eE][+-]?[0-9]+$", "", text).lstrip("+-")
    return "0" in mantissa and not mantissa.strip("0.")

Worth double-checking against openjd_expr::value::text_spells_zero — Rust's regex crate is linear-time by construction, so a pattern that is safe there is not necessarily safe once transliterated to Python re.



def _spells_zero(text: str) -> bool:
return _SPELLS_ZERO.match(text) is not None


def _normalized_range_element(elem: str, to_int: bool) -> Any:
"""The number an ``<intstring>``/``<floatstring>`` range element denotes -- an
``int`` for ``<intstring>``, and for ``<floatstring>`` the plain-notation text
of the value, since ``str(Decimal)`` cannot render every magnitude without an
exponent. Returns the element unchanged when it does not denote a number, or
when its plain-notation form would not fit this field's character cap."""
"""The value an ``<intstring>``/``<floatstring>`` range element denotes.

An ``<intstring>`` becomes an ``int``. A ``<floatstring>`` keeps its text less
redundant leading zeros, so the decimal places it was written with survive
(§7.5). Returns the element unchanged when it does not denote a number.
"""
try:
if to_int:
return int(elem)
value = Decimal(elem)
if not value.is_finite():
# Decimal accepts 'nan'/'inf', which are not <floatstring>s; leave
# them as text rather than changing how they render.
return elem
# Bound the expansion from the exponent before performing it. Decimal
# construction from a string is unbounded -- the context's Emax/Emin
# constrain arithmetic results, not construction, so Decimal('1e999999999')
# is finite and the guard above does not stop it -- and the plain-notation
# length is linear in the exponent, so 11 characters of template text
# would otherwise materialize ~10**9 characters.
#
# The bound is the character cap this field already enforces, because an
# expansion past it cannot be carried here as text at all: the value is
# stored in a TaskRangeList element, whose str member is
# TaskParameterStringValueAsJob, and pydantic's union falls through to
# the numeric members instead, rendering the element as an integer, or as
# inf, or as 0.0. Such an element keeps its source text -- what this
# function already does for anything it cannot normalize, and how it
# rendered before normalization was introduced.
exponent = cast(int, value.as_tuple().exponent) # finite: never 'n'/'N'/'F'
# Length of the '[-]<integer>.<fraction>' text returned below. Stripping
# redundant zeros can only shorten it, so this is an upper bound.
expansion_len = (
(1 if value.is_signed() else 0)
+ max(value.adjusted() + 1, 1) # integer digits, at least one
+ 1 # the decimal point
+ max(-exponent, 1) # fraction digits, at least one
)
if expansion_len > _MAX_TASK_PARAM_VALUE_LEN:
return elem
# format(value, 'f') is exact and plain at every magnitude: with no
# precision in the format spec it neither rounds to getcontext().prec
# nor switches to exponent notation, so an embedding app that sets a
# different context cannot change what this library renders. Neither
# holds of the obvious alternatives -- normalize() rounds to the
# ambient precision, str() switches to exponent notation below 1e-6,
# and quantize() raises InvalidOperation once the result needs more
# digits than the ambient precision allows.
text = format(value, "f")
negative = text.startswith("-")
integer, _, fraction = text.lstrip("-").partition(".")
# A <floatstring> denotes a number, so redundant zeros in its source
# text are not part of the value: '02.50' is 2.5. One fractional digit
# is always kept, matching openjd-rs, which renders the float 1.0 as
# `1.0` and never as `1`.
integer = integer.lstrip("0") or "0"
fraction = fraction.rstrip("0") or "0"
if negative and integer == "0" and fraction == "0":
# The number denoted by '-0.0' is zero, which has no sign; openjd-rs
# renders it `0.0`.
negative = False
return f"{'-' if negative else ''}{integer}.{fraction}"
except (ValueError, ArithmeticError):
# Not a number at all — e.g. a format string that resolved to
# non-numeric text. A literal range is already checked against its
# element type at template parse time, so leave such a value to the
# existing behaviour rather than adding a rejection path here.
return int(elem) # int() already drops leading zeros
# Parsed only to check it is a number; the text is what renders. Not a
# Decimal -- re-rendering one is context-sensitive and unbounded (§7.5).
float(elem)

@github-actions github-actions Bot Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

float() accepts a wider grammar than _REDUNDANT_LEADING_ZEROS normalizes, so for several inputs the §7.5 rule this function exists to implement silently does not apply and the raw text is forwarded to a task command line.

float() (like Decimal(), per PEP 515 and the float() grammar in the language reference) accepts underscore separators and any Unicode Nd digit, not just [0-9]:

  • "1_000.50" parses, the regex matches nothing, and the element renders 1_000.50.
  • "٠٠٧" (Arabic-Indic) parses as 7, but 0+ and the (?=[0-9]) lookahead only see ASCII, so the leading zeros survive and the element renders ٠٠٧.
  • "1." and ".5" parse, and render 1. and .5.

The removed code did not have this gap: format(Decimal(elem), "f") re-rendered in canonical ASCII regardless of how the source was spelled, so "1_000.50" became 1000.5, "٠٠٧" became 7.0, and "1." became 1.0.

These reach here only via a resolved format string — a literal is coerced by the template model's FloatRangeList, whose Decimal member matches first — so the blast radius is limited to range: "{{Param.X}}"-style resolution. But that is exactly the path the validator was moved to the instantiation target to cover, and the values do reach --flag {{Task.Param.P}}.

If the intent is "keep the author's text", then the leading-zero rule should still apply uniformly to it; the simplest way to keep parse and normalize in the same grammar is to gate on an explicit <floatstring> regex (sign, ASCII digits, optional point/exponent) rather than on float() not raising, and reject/pass-through anything else. As written, whether zeros are stripped depends on which script the digits are written in.

except ValueError:
# A resolved format string can be non-numeric. Literals are checked at
# template parse time, so carry it through rather than rejecting here.
return elem
# Trimmed because the text reaches a command line and float() ignores
# surrounding whitespace; openjd-rs trims here too.
text = _REDUNDANT_LEADING_ZEROS.sub(r"\1", elem.strip())
# Zero has no sign, so drop one without dropping the decimal places: '-0.00'
# renders `0.00`. Decided from the text, not from `value` -- the parse
# underflows to 0.0 below ~5e-324, and a tiny value's digits are exactly what
# a <floatstring> is for.
return text.lstrip("+-") if _spells_zero(text) else text


# Target model for task parameters when instantiating a job.
Expand All @@ -1327,22 +1297,14 @@ class RangeListTaskParameterDefinition(OpenJDModel_v2023_09):
@field_validator("range", mode="before")
@classmethod
def _normalize_numeric_range_elements(cls, value: Any, info: ValidationInfo) -> Any:
# §3.4.1.1/§3.4.1.2: an <IntRangeList> element is `<integer> |
# <intstring>` and a <FloatRangeList> element is `<float> |
# <floatstring>`, where an <intstring>/<floatstring> is "a string whose
# value is the string representation of" a number (§2.3, §2.4). Such an
# element therefore denotes that number, not its source text, so '02'
# is the task value 2 and '02.50' is 2.5. Python was keeping the source
# text, which reaches a task command line as `--frame 02`.
#
# Applied on the instantiation target so every inbound range path is
# covered: a literal list, a range-expression expansion, and an RFC 0006
# typed whole-field resolution all funnel through this model.
# §7.5: a string-form range element loses redundant leading zeros and
# keeps its decimal places, so '02' is the value 2 and '02.50' renders
# `2.50`. Numeric literals carry no such request and are left as parsed;
# STRING and PATH ranges have no numeric form at all.
#
# Numeric elements are left exactly as parsed. A FLOAT range of [1.0]
# renders 1.0, which openjd-rs also does and the conformance suite pins
# (base/jobs/3.4--float-parameter). STRING and PATH ranges are text by
# definition and are never touched.
# On the instantiation target so all three inbound paths are covered: a
# literal list, a range-expression expansion, and RFC 0006 whole-field
# resolution.
param_type = info.data.get("type")
if not isinstance(value, list) or param_type not in (
TaskParameterType.INT,
Expand Down
4 changes: 2 additions & 2 deletions test/openjd/model_v0/test_step_param_space_iter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,8 @@ def test_int_range_intstring_elements(self) -> None:

def test_float_range_floatstring_elements(self) -> None:
# WHEN a FLOAT range is written with <floatstring> elements
# THEN each task gets the number the element denotes
assert self._task_values("FLOAT", ["1.5", "02.50"]) == ["1.5", "2.5"]
# THEN each task gets the element's text, less redundant leading zeros
assert self._task_values("FLOAT", ["1.5", "02.50", "3.500"]) == ["1.5", "2.50", "3.500"]

def test_float_range_numeric_literals_keep_their_scale(self) -> None:
# A <float> literal is not a string representation, so it keeps the scale
Expand Down
135 changes: 62 additions & 73 deletions test/openjd/model_v0/v2023_09/test_parameter_space.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,13 +441,12 @@ def test_list_form_range_is_still_capped(self, model: Any, obj: dict[str, Any])


class TestRangeListElementNormalization:
"""§3.4.1.1/§3.4.1.2: an `<IntRangeList>` element is `<integer> | <intstring>`
and a `<FloatRangeList>` element is `<float> | <floatstring>`. An
`<intstring>`/`<floatstring>` is "a string whose value is the string
representation of" a number (§2.3, §2.4), so it denotes that number and not
its source text — a range of `['1', '02', '003']` is the task values 1, 2 and
3. These values reach a task command line, so keeping the text renders
`--frame 02`.
"""§7.5 on a range element: leading zeros go, decimal places stay.

`['1', '02', '003']` is the task values 1, 2 and 3; keeping the text renders
`--frame 02`. But `'2.50'` renders `2.50`, because the string form is how a
template asks for a fixed number of decimal places —
`EXPR/jobs/expr1.3.4--float-passthrough` pins that for a FLOAT default.
"""

@pytest.mark.parametrize(
Expand All @@ -471,96 +470,86 @@ def test_intstring_elements_carry_their_value(self, param_type: str) -> None:
@pytest.mark.parametrize(
"element,expected",
(
pytest.param("1.5", "1.5", id="no normalization needed"),
pytest.param("02.50", "2.5", id="leading and trailing zeros"),
pytest.param("3.500", "3.5", id="trailing zeros"),
pytest.param("-02.50", "-2.5", id="negative"),
# An integral float keeps one fractional digit. openjd-rs renders
# the <floatstring> '1.0' as `1.0` and '007' as `7.0`, so dropping
# the fraction would make the same number render two ways depending
# on whether it was written as a <float> or a <floatstring>.
pytest.param("1.0", "1.0", id="integral float"),
pytest.param("1.5", "1.5", id="nothing to strip"),
# The rule, in both directions at once.
pytest.param("02.50", "2.50", id="leading zero goes, trailing zero stays"),
pytest.param("3.500", "3.500", id="trailing zeros stay"),
pytest.param("-02.50", "-2.50", id="negative"),
pytest.param("+02.50", "+2.50", id="explicit plus sign is kept"),
pytest.param("007", "7", id="leading zeros on a whole number"),
Comment thread
leongdl marked this conversation as resolved.
pytest.param("100", "100", id="a zero that is a significant digit"),
# Neither the zero before the point nor the last of an all-zero
# integer part is redundant.
pytest.param("0.50", "0.50", id="the necessary leading zero stays"),
pytest.param("000", "0", id="all zeros keeps one"),
pytest.param("0", "0", id="bare zero"),
pytest.param("0.0", "0.0", id="zero"),
pytest.param("0.00", "0.00", id="zero keeps its trailing zeros too"),
# Zero has no sign, which openjd-rs and mainline both render away.
# Dropping it must not drop the decimal places with it.
pytest.param("-0.0", "0.0", id="negative zero has no sign"),
pytest.param("007", "7.0", id="leading zeros on a whole number"),
pytest.param("100", "100.0", id="whole number"),
# Exponent notation must never reach a task command line, at any
# magnitude and in either direction.
pytest.param("1E+2", "100.0", id="exponent notation in the source"),
pytest.param(
"0.0000001",
"0.0000001",
id="below 1e-6, where Decimal.__str__ would give 1E-7",
),
pytest.param(
"1E+30",
"1" + "0" * 30 + ".0",
id="more digits than the default decimal precision",
),
pytest.param("-0.00", "0.00", id="unsigned, but still two places"),
# An all-zero mantissa spells zero whatever the exponent.
pytest.param("0E+2", "0E+2", id="exponent form of zero"),
pytest.param("-0e5", "0e5", id="signed exponent form of zero"),
# Underflow is not zero being spelled: the digits are the request.
pytest.param("1e-400", "1e-400", id="underflow keeps its digits"),
pytest.param("0." + "0" * 400 + "1", "0." + "0" * 400 + "1", id="tiny plain decimal"),
# float() ignores surrounding whitespace and this text reaches a
# command line, so it is trimmed rather than forwarded.
pytest.param(" 1.5 ", "1.5", id="surrounding whitespace"),
pytest.param("1.0", "1.0", id="integral float keeps its point"),
# Exponent notation is the author's own text and is forwarded. The
# FLOAT parameter default path does the same for `default: "1E+2"`.
pytest.param("1E+2", "1E+2", id="exponent notation"),
Comment thread
leongdl marked this conversation as resolved.
pytest.param("01E+2", "1E+2", id="leading zero on an exponent form"),
pytest.param("0.0000001", "0.0000001", id="below 1e-6"),
pytest.param(
"1.2345678901234567890123456789012345678901",
"1.2345678901234567890123456789012345678901",
id="more significant digits than the default decimal precision",
id="more significant digits than a float can hold",
),
),
)
def test_floatstring_elements_carry_their_value(self, element: str, expected: str) -> None:
def test_floatstring_elements_keep_their_scale(self, element: str, expected: str) -> None:
# WHEN the instantiation target parses a float range holding a <floatstring>
model = _parse_model(
model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]}
)

# THEN the element renders as the number it denotes
# THEN it renders as written, less any redundant leading zeros
assert [str(v) for v in model.range] == [expected]

@pytest.mark.parametrize(
"element",
(
# Decimal's context bounds arithmetic results, not construction from
# a string, so these are finite: 11 characters of template text
# whose plain-notation expansion is ~10**9 characters.
pytest.param("1e999999999", id="huge positive exponent"),
pytest.param("1e-999999999", id="huge negative exponent"),
# One character past the cap in each direction: both expand to 1025.
pytest.param("1E+1022", id="one past the cap, positive exponent"),
pytest.param("1E-1023", id="one past the cap, negative exponent"),
),
)
def test_element_too_long_to_expand_keeps_its_source_text(self, element: str) -> None:
# WHEN the instantiation target parses a <floatstring> whose plain-notation
# form would not fit TaskParameterStringValueAsJob's 1024-character cap
model = _parse_model(
model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]}
)

# THEN it keeps its source text rather than being expanded. Expanding it
# would allocate the whole expansion, and the result could not be carried
# as text here anyway: it fails the str member of TaskRangeList's union,
# and pydantic then falls through to the numeric members, rendering the
# element as an integer, or as inf, or as 0.0.
assert [str(v) for v in model.range] == [element]

@pytest.mark.parametrize(
"element,expected",
(
pytest.param("1E+1021", "1" + "0" * 1021 + ".0", id="positive exponent"),
pytest.param("1E-1022", "0." + "0" * 1021 + "1", id="negative exponent"),
# An exponent too large for a float still costs only its own
# characters, which is the whole point: nothing expands it.
pytest.param("1e999999999", "1e999999999", id="huge positive exponent"),
pytest.param("1E+1022", "1E+1022", id="positive exponent past the field cap"),
# Too small for a float either way, and the digits still stand:
# underflowing to 0.0 is a parse limit, not the author writing zero.
pytest.param("1e-999999999", "1e-999999999", id="huge negative exponent"),
pytest.param("1E-1023", "1E-1023", id="negative exponent past the field cap"),
),
)
def test_element_at_the_cap_is_still_normalized(self, element: str, expected: str) -> None:
# GIVEN a <floatstring> whose plain-notation form is exactly 1024
# characters — at the cap, not past it
assert len(expected) == 1024

# WHEN the instantiation target parses it
def test_a_huge_exponent_costs_only_its_own_characters(
self, element: str, expected: str
) -> None:
# WHEN 11 characters of template text denote a number needing ~10**9
# digits in plain notation
model = _parse_model(
model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]}
)

# THEN the length bound has not cost it its normalization
# THEN nothing expands it, so no bound on the exponent is needed to stop a
# template allocating ~10**9 characters. Long *literal* text can still
# exceed TaskParameterStringValueAsJob's cap and fall through to a numeric
# member of the union, as it does on mainline and in 0.11.6; only the
# expansion is gone.
assert [str(v) for v in model.range] == [expected]

def test_floatstring_normalization_ignores_the_decimal_context(self) -> None:
def test_floatstring_rendering_ignores_the_decimal_context(self) -> None:
# GIVEN an embedding application that has narrowed the process-wide
# decimal context, and a <floatstring> with more significant digits
# than that precision allows
Expand All @@ -573,8 +562,8 @@ def test_floatstring_normalization_ignores_the_decimal_context(self) -> None:
model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]}
)

# THEN the value is unrounded: what this library renders is a property of
# the template, not of the host application's decimal context
# THEN it is unrounded: the rendering is a property of the template, not
# of the host application's decimal context
assert [str(v) for v in model.range] == [element]

@pytest.mark.parametrize(
Expand Down
Loading