fix(schema): make the exported schema accept what the engine accepts - #463
Merged
Merged
Conversation
…ibutes `every_serde_alias_in_the_sources_is_reachable_from_the_exported_schema` was green while the schema rejected `"type": "card"`, which every example in the repository writes. Its scanner read the sources line by line and kept a line only if it both started with `#[serde(` and carried an `alias = "`. rustfmt splits an attribute with six arguments across six lines, so the one declaring `container`, `card`, `flex`, `grid` and `positioned` as spellings of `div` was invisible to it. `progress_bar`, which fits on one line, was caught — hence a test that looked like it worked. The scanner now takes each `#[serde(...)]` as a whole, paren-matched across lines. That alone turns it red on the five. The reachability check also had to learn that a variant alias widens an `enum` array rather than adding a sibling property, which is why it demanded `card` next to `div` under `properties` and would have stayed red after the real fix. Refs #424
…lizers `AnimationTiming` and `schema::style::FontWeight` both parse through a hand-written Deserialize and both derive JsonSchema, so the exported schema described the Rust shape rather than the JSON the engine takes. Measured against the repository's own examples, these two accounted for 52 of the 62 rejected paths. AnimationTiming deserializes through AnimationTimingWire. Three divergences followed, and the third is the one that costs a user a render: `loop` was declared required although the wire defaults it; `loop` was declared a boolean although it also takes a play count (#330); and `repeat_count`, which is an output of RepeatSpec::into_parts rather than a wire field, was advertised as a property — the wire type is deny_unknown_fields, so a generator that believed the schema got its whole component dropped. AnimationTiming derives neither Serialize nor Deserialize, so its serde attributes are read by schemars alone and changing them cannot move runtime behaviour. FontWeight's visitor takes "normal", "bold" or a number; the derive emitted "Normal", "Bold" and {"Weight": n}. It now carries a hand-written JsonSchema saying what the visitor does, which is also where its description had to move: a doc comment on a type that no longer derives JsonSchema stops being interface text. Both are pinned by a test that asserts the schema and serde agree on the same literals rather than asserting the schema alone. Refs #424
A templated scenario writes `"delay": "$delay"` where the schema declares a number, so a Draft-7 validator rejected it although the engine substitutes before deserializing and accepts the file. Four of the fifteen examples this repository ships are templated. Every position declaring a number, an integer or a boolean now also accepts a string matching `^\$[A-Za-z_][A-Za-z0-9_]*$`, hoisted into a single `TemplatePlaceholder` definition — inlining it at each of the several thousand sites cost 690KB. Two things are deliberately not widened. A node carrying an `enum` is left alone, which keeps the `type` tag a closed set: it is the discriminator a validator needs to report inside the right branch, and a free string there would cost that for every component. And the pattern means a bare string is still refused where a number is expected, so `"delay": "soon"` stays an error rather than becoming one more thing the schema waves through. The exported schema grows 1.11MB to 1.42MB. A test compiles what `rustmotion schema` actually prints and validates every example against it, so the claim is measured on the artifact rather than on the function that builds it. Closes #424
…eal fault A one-letter typo in an animation name produced a 3611-node error tree whose best match was "is not valid under any of the given schemas", anchored on the whole component. Nothing in that names `style/animation/0/name`, and a generator reading it cannot act on it. Each branch already pinned its tag to a one-element enum, so the tag was never what was missing — the steering was. A union whose branches all pin one key to disjoint values becomes an `allOf` of `if`/`then` on that key, which leaves validation identical and makes a validator descend into the one branch that was meant. It applies to any union shaped that way, which is ComponentBase's 54 variants and AnimationEffect's 59, among others. The component/`for-each`/`use` union has no common tag, so it is steered on key presence instead, in the code that builds it. The same typo now reports at `scenes/0/children/0/style/animation/0/name` with the list of names it could have been, in a tree of 10 nodes. A `for-each` missing its `template` says so, and `propz` instead of `props` is named. Refs #424
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rustmotion schemais what an editor and a generating model read to know what to write, and it rejected every one of the fifteen scenarios this repository ships as examples. Draft-7, 62 failing paths. All fifteen validate now, and the one-letter typo that used to produce a 3611-node error tree reports in 10 nodes at the property that is wrong.#424 named two causes and measured both. Measuring the rest turned up two more, so this is four fixes rather than two.
The templating vocabulary. A templated scenario writes
"delay": "$delay"where the schema declares a number; the engine substitutes before it deserializes, so the file is valid input. Every position declaring a number, an integer or a boolean now also accepts^\$[A-Za-z_][A-Za-z0-9_]*$, through a singleTemplatePlaceholderdefinition — inlining it at the several thousand sites cost 690KB where the$refcosts 310KB. A node carrying anenumis deliberately left alone: that keepstypea closed set, which is the discriminator the second half depends on. And the pattern means"delay": "soon"is still an error, rather than one more thing the schema waves through.div's five aliases. The schema did not knowcard,flex,grid,containerorpositioned, which the examples use throughout.every_serde_alias_in_the_sources_is_reachable_from_the_exported_schemawas green the whole time: its scanner kept a source line only if that line both started with#[serde(and carried analias = ", and rustfmt splits a six-argument attribute across six lines.progress_barfits on one line and was caught, which is what made the test look like it worked. The scanner now takes each attribute whole, paren-matched across lines, and the reachability check knows a variant alias widens anenumrather than adding a sibling property.Two hand-written deserializers describing their Rust shape. These were not in the issue and were 52 of the 62.
AnimationTimingparses throughAnimationTimingWire:loopwas declared required although the wire defaults it, declared a boolean although it also takes a play count (#330), andrepeat_countwas advertised as a property although it is an output ofRepeatSpec::into_partsand the wire type isdeny_unknown_fields— believing the schema there cost you the whole component.FontWeight's visitor takes"normal","bold"or a number; the derive emitted"Normal","Bold"and{"Weight": n}.AnimationTimingderives neitherSerializenorDeserialize, so its serde attributes are read by schemars alone and changing them cannot move runtime behaviour. Six more types have the same shape and are unverified — #462.Steering. Every branch already pinned its tag to a one-element enum, so the tag was never what was missing. A union whose branches pin one key to disjoint values becomes an
allOfofif/thenon that key: validation is unchanged, and a validator descends into the branch that was meant instead of failing all 54 and reporting whichever it liked. The component/for-each/useunion has no common tag and is steered on key presence.fade_in_uppnow reports atscenes/0/children/0/style/animation/0/name; afor-eachwithout itstemplatesays so;propzinstead ofpropsis named.The decision #424 left open — whether the schema should validate a templated file or only an expanded one — is settled here in favour of templated. The README sells the schema for editor autocompletion and LLM prompts, four of the fifteen examples are templated, and the skill tells a generator to factorise.
boonjoins as a dev-dependency, for the test that compiles what the command actually prints and validates every example against it. It locks four new packages againstjsonschema's seventy-seven, andcargo auditwith CI's ignore list reports nothing new.Verification —
cargo fmt --all --check,cargo clippy --workspace --all-targets --features rustmotion/studio -- -D warnings,cargo test --workspace --features rustmotion/studio: 0 failed. The example test was confirmed red with the widening removed. The 62 → 0 figure was measured twice through different validators, Pythonjsonschema4.26 while working andboon0.6 in the committed test.Closes #424
Refs #462