Skip to content

Fixed silent data loss when parsing DCC documents - #13

Open
HassanAkbar wants to merge 5 commits into
mainfrom
fix/parse-data-loss
Open

Fixed silent data loss when parsing DCC documents#13
HassanAkbar wants to merge 5 commits into
mainfrom
fix/parse-data-loss

Conversation

@HassanAkbar

Copy link
Copy Markdown
Member

Reading a DCC file and writing it back silently dropped measurements. This fixes the six causes.

On dcclib/valid.xml, 8 of 13 quantities and the entire measurement list disappeared,
and the document still validated. The test suite stayed green because
round_trip_spec.rb only asserted the output was a String containing one word.

What was wrong:

  • Dcc::V3 overwrote its own :list model with the D-SI one when re-exporting types.
  • D-SI v1 expandedUnc and coverageInterval were never wired into si:real.
  • dcc:mathml was mapped onto Mml::V3::Math directly instead of through its
    dcc:xmlType wrapper, so it never parsed.
  • DecimalXmlList serialized one list into N elements holding Ruby array literals.
  • A Schematron rule rejected the single-uncertainty broadcast that PTB's own reference
    documents use.

Breaking: formula.mathml returns a Dcc::V2::Mathml / Dcc::V3::Mathml wrapper
instead of an Mml::V3::Math. Callers now read formula.mathml.math.

Known and accepted:

  • lib/dcc/diff.rb is touched although it is not a parsing fix. Dcc::Diff raised
    NoMethodError on the new value object; it now treats anything it cannot descend
    into as a leaf, which closes the class rather than special-casing one type.
  • Values#== accepts a plain Array but eql? does not. That is deliberate: eql? is
    what Hash uses and must stay strict.
  • Quantities nested under dcc:statement or measurementMetaData are still dropped —
    dcc:data is unmapped on Dcc::Base::Statement. Separate bug, documented in the spec.
  • Four inline rubocop:disable in the D-SI mapping modules. Around twenty sibling
    modules exceed the same metrics and are exempted by file; keeping these four shaped
    like their siblings beats making them diverge.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes multiple causes of silent data loss when round-tripping DCC documents by correcting model registrations/mappings (DCC vs D-SI and MathML) and by fixing decimal XML list serialization, backed by new fidelity-focused specs.

Changes:

  • Prevent D-SI type re-export from overwriting DCC’s own :list registration and add missing D-SI expandedUnc / coverageInterval wiring under si:real.
  • Introduce dcc:mathml as a proper dcc:xmlType wrapper model (breaking API change: formula.mathml.math).
  • Fix decimal XML list casting/serialization to preserve space-separated wire form and avoid scientific notation leakage; expand tests to catch regressions.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/parse_fidelity_spec.rb Adds end-to-end round-trip element-count assertions to detect silent parse/serialize data loss.
spec/dcc/validate/schematron_spec.rb Adds specs for broadcast-vs-per-value uncertainty list consistency validation.
spec/dcc/type_spec.rb Adds tests for DecimalXmlList value-object behavior (equality, enumeration, YAML, attribute round-trip).
spec/dcc/convert/additional_spec.rb Ensures CSV conversion doesn’t emit scientific notation for list values.
lib/dcc/validate/schematron/rules/uncertainty_consistency.rb Extends rule to validate multiple uncertainty-related lists and allow broadcast semantics.
lib/dcc/v3/mathml.rb Adds DCC v3 Mathml wrapper model registered as :mathml.
lib/dcc/v3.rb Autoloads/registers Mathml; prevents D-SI re-export from overwriting DCC-owned model IDs (notably :list).
lib/dcc/v2/mathml.rb Adds DCC v2 Mathml wrapper model registered as :mathml.
lib/dcc/v2.rb Autoloads/registers Mathml in v2 element set.
lib/dcc/type/decimal_xml_list.rb Reworks decimal XML list type into a value object to preserve wire format and correct serialization.
lib/dcc/si/v2/expanded_unc.rb Adds D-SI v2 ExpandedUnc model registered as :expandedUnc.
lib/dcc/si/v2/coverage_interval.rb Adds D-SI v2 CoverageInterval model registered as :coverageInterval.
lib/dcc/si/v2.rb Autoloads/registers new v2 uncertainty models.
lib/dcc/si/v1/expanded_unc.rb Refactors v1 ExpandedUnc to share base mapping module.
lib/dcc/si/v1/coverage_interval.rb Adds D-SI v1 CoverageInterval model registered as :coverageInterval.
lib/dcc/si/v1.rb Autoloads/registers v1 CoverageInterval.
lib/dcc/si/base/real.rb Maps deprecated expandedUnc and coverageInterval under si:real.
lib/dcc/si/base/expanded_unc.rb Introduces shared mapping for si:expandedUncType.
lib/dcc/si/base/coverage_interval.rb Introduces shared mapping for si:coverageIntervalType.
lib/dcc/si/base.rb Autoloads shared uncertainty base modules.
lib/dcc/diff.rb Adjusts diff “primitive” detection to treat unknown value objects as leaf nodes.
lib/dcc/convert/csv.rb Uses decimal list wire serialization for CSV output to avoid scientific notation.
lib/dcc/base/mathml.rb Introduces shared DCC dcc:mathml wrapper mapping around ml:math.
lib/dcc/base/formula.rb Changes formula.mathml from Mml::V3::Math to :mathml wrapper type.
CHANGELOG.adoc Documents fixes and the breaking change for MathML wrapper behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/dcc/diff.rb

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (1)

spec/parse_fidelity_spec.rb:52

  • The new round-trip assertions for dcc:list only verify that the wrapper elements survive. If a regression causes dcc:list to parse as an empty list again, the list count can still match while nested dcc:quantity elements are silently dropped (which is the failure mode described for dcclib/valid.xml). Consider also asserting quantity counts for dcclib/valid.xml to lock in the original bug fix.
  describe "dcc:quantity" do
    it "round-trips every quantity in dcc_excel/example.xml" do
      expect(counts("dcc_excel/example.xml", "quantity")).to eq(in: 20, out: 20)
    end
  end

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

@HassanAkbar
HassanAkbar marked this pull request as ready for review July 31, 2026 10:28
@HassanAkbar
HassanAkbar requested a review from ronaldtse July 31, 2026 10:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants