Fix parsed models dropping post-parse changes - #743
Conversation
JS build checkTriggered [ The result will appear as a |
There was a problem hiding this comment.
Pull request overview
This PR fixes a long-standing divergence where XML-parsed models could silently drop post-parse mutations during to_xml because serialization relied on the parse-time element_order as the source of truth. It introduces reconciliation logic to align element_order with the model’s current values (without mutating the stored order) and adds comprehensive regression coverage.
Changes:
- Add
OrderReconcilerto insert missingelement_orderentries for post-parse mutations while preserving idempotent serialization. - Improve rule compilation and ordered application to correctly handle element name aliases (including delegated mappings) and collection indexing keyed by rule identity.
- Ensure parsed model
element_orderis mutable by duplicating the frozen DOM order cache, and add extensive regression specs covering mutations after parse.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/lutaml/model/parsed_model_mutation_spec.rb | Adds regression specs covering post-parse mutations and expected serialization behavior. |
| lib/lutaml/xml/transformation/rule_compiler.rb | Propagates alias names into compiled rules for delegated element mappings. |
| lib/lutaml/xml/transformation/ordered_applier.rb | Uses reconciled order + per-rule collection counters; restricts “remaining rule” emission to reconciler-provided fallbacks. |
| lib/lutaml/xml/transformation/order_reconciler.rb | New reconciliation module to insert missing element_order entries and provide fallback rules. |
| lib/lutaml/xml/transformation.rb | Includes OrderReconciler into XML transformation pipeline. |
| lib/lutaml/xml/transformation_support.rb | Autoloads OrderReconciler. |
| lib/lutaml/xml/model_transform.rb | Duplicates doc.root.order to avoid freezing element_order on parsed models. |
| lib/lutaml/model/serialize/builder.rb | Removes now-unused order_tracking_enabled? helper. |
| lib/compat/opal/lutaml_model_boot.rb | Requires the new reconciler in the Opal boot path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JS build checkTriggered [ The result will appear as a |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lib/lutaml/xml/transformation/order_reconciler.rb:93
classify_rulesonly inserts deficits for unambiguous rules. For an element rule that shares its serialized name with another rule (ambiguous), the first matching rule can still havecoverage[rule] > 0; if its collection grows after parse / via in-place mutation,missingbecomes positive but the rule is neither inserted nor emitted as a fallback, so the extra items are silently dropped during serialization. Consider treating an already-covered rule as safe to reconcile (inserting additional entries after its last resolved occurrence).
if reconcilable?(rule, options) &&
unambiguous?(rule, element_rules)
deficits[rule] = missing
elsif coverage[rule].zero? &&
emit_uncovered?(rule, element_rules, model_instance)
JS build checkTriggered [ The result will appear as a |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
lib/lutaml/xml/transformation/order_reconciler.rb:95
classify_rulesonly inserts deficits when the rule is unambiguous. For rules that are ambiguous by serialized name/namespace but already have someelement_ordercoverage (i.e., they’re the dispatcher “winner” during round-trip), growing the collection still producesmissing > 0but the rule is neither inserted nor returned as a fallback, so the newly-added items will still be dropped during ordered serialization.
if reconcilable?(rule, options) &&
unambiguous?(rule, element_rules)
deficits[rule] = missing
elsif coverage[rule].zero? &&
emit_uncovered?(rule, element_rules, model_instance)
fallback << rule
end
A model parsed from XML behaved differently from one built in memory.
Parsed models keep the
element_ordercaptured at parse time.Anything you set afterwards never got into it.
So
to_xmldropped the value, with no error.Bugs fixed:
element_orderwas frozen, so<<raisedFrozenError.Two output changes worth knowing:
Klass.new(attrs) { ... }now emits in declaration order.render_empty.