fix(runtime): coerce integer scalars to float at boxing time - #104
Conversation
Number fields declared as float/double/decimal kept their raw JSON representation when boxed by load_json, so a value authored server-side (114.0) and the same value round-tripped through a layer with no int/float distinction (114) compared unequal in diff/equals and produced a spurious update delta. Coerce an integer JSON scalar to f64 at the single boxing chokepoint when the slot's range is a real-valued number type, so the value is canonical before any diff/equals/patch sees it. Detection prefers the resolved RDF datatype IRI (also catches subtypes of float/double/decimal) and falls back to the builtin LinkML type names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| let Some(slot) = slot else { | ||
| return value; | ||
| }; | ||
| if !slot.is_range_floating_point() { |
There was a problem hiding this comment.
what about the opposite ? this method is rather hard to read (despite it being simple) i was basically expecting a structure:
- for expected range float
- coerce this into float
- coerce that into float
- for expected range int
- coerce this into int
- coerce that into int
the question is: is this really only happening with ints that should be coerced into floats, or can it also be the other way around, and could lit even happen with other scalar ranges ?
There was a problem hiding this comment.
It only happens for ints -> floats indeed. But I've let Claude rewrite it to be more readable 👍
| if self.range_class.is_some() || self.range_enum.is_some() { | ||
| return false; | ||
| } | ||
| matches!(self.e.range(), Some("float" | "double" | "decimal")) |
There was a problem hiding this comment.
not great but i guess there is no other way to do this.
There was a problem hiding this comment.
This is only a fallback. It tries to use xsd:int to derive the type, but as a fallback it looks at the range. Added a comment to explain, and moved the magic strings to a constant.
…nges Address review: structure coercion explicitly per expected range kind and handle the symmetric integer direction (whole float -> int), guarded to exactly-integral in-range values. Hoist builtin type names into consts and share the name-fallback resolver. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 058d13c9b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !n.is_f64() { | ||
| if let Some(f) = n.as_f64().and_then(serde_json::Number::from_f64) { | ||
| return JsonValue::Number(f); |
There was a problem hiding this comment.
When a decimal/double/float-ranged slot receives an integer JSON number outside f64's exact integer range, this branch converts it through as_f64() before storing it. For example, a valid decimal value like 9007199254740993 is boxed as the f64 value 9007199254740992.0, so parsing mutates the user's data before validation/diff/serialization. This is especially problematic because decimal is included in is_range_floating_point(); skip coercion unless the integer is exactly representable, or avoid f64 coercion for decimal ranges.
Useful? React with 👍 / 👎.
| if f.fract() == 0.0 && (i64::MIN as f64..=i64::MAX as f64).contains(&f) { | ||
| Some(f as i64) |
There was a problem hiding this comment.
For an integer-ranged slot with a whole float at the upper boundary, e.g. JSON 9223372036854775808.0, the range check passes because i64::MAX as f64 rounds up to that same f64 value; the following as i64 cast then saturates to 9223372036854775807. That silently changes an out-of-range user value instead of leaving it intact for validation as the comment promises, so the bound check needs to exclude values above the largest exactly castable i64.
Useful? React with 👍 / 👎.
2 participants
Number fields declared as float/double/decimal kept their raw JSON representation when boxed by load_json, so a value authored server-side (114.0) and the same value round-tripped through a layer with no int/float distinction (114) compared unequal in diff/equals and produced a spurious update delta.
Coerce an integer JSON scalar to f64 at the single boxing chokepoint when the slot's range is a real-valued number type, so the value is canonical before any diff/equals/patch sees it. Detection prefers the resolved RDF datatype IRI (also catches subtypes of float/double/decimal) and falls back to the builtin LinkML type names.