Audit follow-ups: TODO #1 (alias spec), #2 (sharded warning), #3 (kebab-case rename) - #27
Merged
Conversation
Audit item A6 in data-private/TODO.full-cdd/17 hypothesized a collision where two REGISTRY entries share an alias and alias_map silently keeps whichever was inserted last. As of this snapshot, no such collision exists in the REGISTRY: MDC_P021 => aliases %w[definition_class] MDC_P023 => aliases %w[unit_structure unit_symbol] MDC_P025_1 => aliases %w[symbol preferred_symbol_text symbol_in_text] The new spec/property_id_aliases_spec.rb pins two invariants: 1. No alias is claimed by multiple REGISTRY IDs (the hypothetical collision would fail this assertion). 2. Each documented alias resolves to its canonical ID via PropertyIds.canonical_id. The audit premise was based on stale state (the MDC_P021/MDC_P023 entries never shared the 'symbol' alias in this repo). The spec is still valuable as a regression guard against future drift.
Audit item A7 in data-private/TODO.full-cdd/17 flagged that the sharded_dir_reader_spec.rb was weakened to only assert 'database.classes.size == 26' because the iec63213 fixture has header-only PROPERTY/VALUELIST/VALUETERMS exports (0 data rows). The spec silently masks that data-quality issue. Add a before(:all) hook that classifies every export_*.xls in the fixture as data-bearing or header-only and warns on stderr naming the empty export types. The spec assertions still pass either way; the warning is a visibility nudge so a stale fixture does not silently mask data-quality regressions. The warning fires only when the fixture is present (the existing skip guard is preserved). The spreadsheet gem is used (same as WorkbookReader::SpreadsheetSource in production) rather than Roo, which fails to load the .xls files in the iec63213 fixture.
Companion to data-private PR #13 and opencdd.github.io PR (TBD). Renames these dict-key references in lib comments and spec fixture paths: iec61360-4 -> iec-61360-4 iec61360-7 -> iec-61360-7 iec61987 -> iec-61987 (no occurrences in this repo) iec62683 -> iec-62683 iec62720 -> iec-62720 (no occurrences in this repo) iec63213 -> iec-63213 iec63508 -> iec-63508 (no occurrences in this repo) The standalone `iec61360` key (no -4/-7 suffix) is NOT renamed — it is its own dict key, not a member of the rename set. Touches only docstring examples and skip-gated fixture paths. No code semantics change. The full rspec suite passes (the fixture-gated specs skip cleanly when downloads/ is absent).
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
1 participant
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.
Summary
Three small audit follow-ups from TODO.complete/, on one branch as
three atomic commits.
TODO #1 — alias registry regression spec
spec/property_id_aliases_spec.rbpins two invariants:Audit item A6 hypothesized a MDC_P021/MDC_P023 collision on
symbol;the current REGISTRY has no such collision (MDC_P021 owns
definition_class, MDC_P023 ownsunit_structure unit_symbol,MDC_P025_1 owns
symbol). The spec is still valuable as a regressionguard against future drift.
TODO #2 — sharded-dir reader spec warning hook
spec/parcel/sharded_dir_reader_spec.rbadds abefore(:all)hookthat classifies every export_*.xls in the iec63213 fixture as
data-bearing or header-only and warns on stderr when the fixture
is poor. The spec assertions still pass; the warning is a visibility
nudge so a stale fixture does not silently mask data-quality
regressions.
Uses the
spreadsheetgem (not Roo) — same asWorkbookReader::SpreadsheetSource in production.
TODO #3 — kebab-case rename for 7 IEC dict keys
Companion to data-private PR #13. Renames dict-key references in
lib comments and spec fixture paths:
(iec61987, iec62720, iec63508 had no occurrences in this repo.)
The standalone
iec61360key is NOT renamed (it is its own dictkey, not part of the rename set).
Touches only docstring examples and skip-gated fixture paths. No
code semantics change.
Test plan
bundle exec rspec— full suite passes; fixture-gated specsskip cleanly when downloads/ is absent.
spec/property_id_aliases_spec.rb— 8 examples, 0 failures.spec/parcel/sharded_dir_reader_spec.rb— 10 examples pass(10 pending when fixture absent, 0 failures either way).